Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-22 Thread Igor Mammedov 
On Tue, 22 Mar 2016 11:22:56 +1100
David Gibson  wrote:

> On Mon, Mar 21, 2016 at 11:43:34AM +0100, Igor Mammedov wrote:
> > On Mon, 21 Mar 2016 14:57:53 +1100
> > David Gibson  wrote:
> >   
> > > On Fri, Mar 18, 2016 at 08:59:32AM +0530, Bharata B Rao wrote:  
> > > > On Thu, Mar 17, 2016 at 09:03:43PM +1100, David Gibson wrote:
> > > > > On Wed, Mar 16, 2016 at 04:48:50PM +0100, Igor Mammedov wrote:
> > > > > > On Wed, 16 Mar 2016 09:18:03 +0530
> > > > > > Bharata B Rao  wrote:
> > > > > > 
> > > > > > > On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote:
> > > > > > > > On Fri, 11 Mar 2016 10:24:29 +0530
> > > > > > > > Bharata B Rao  wrote:
> > > > > > > >   
> > > > > > > > > Hi,
> > > > > > > > > 
> > > > > > > > > This is the next version of "Core based CPU hotplug for 
> > > > > > > > > PowerPC sPAPR" that
> > > > > > > > > was posted at
> > > > > > > > > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> > > > > > > > > 
> > > > > > > > > device_add semantics
> > > > > > > > > 
> > > > > > > > > For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> > > > > > > > > (qemu) device_add 
> > > > > > > > > spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]
> > > > > > > > >   
> > > > > > > > do you plan to allow user to hotplug different cpu_models?
> > > > > > > > If not it would be better to hide cpu_model from user
> > > > > > > > and set it from machine pre_plug handler.  
> > > > > > > 
> > > > > > > In my earlier implementations I derived cpu model from -cpu and 
> > > > > > > threads from
> > > > > > > -smp,threads= commandline options and never exposed them to 
> > > > > > > device_add
> > > > > > > command.
> > > > > > > 
> > > > > > > Though we don't support heterogenous systems (different cpu 
> > > > > > > models and/or
> > > > > > > threads) now, it was felt that it should be easy enough to 
> > > > > > > support such
> > > > > > > systems if required in future, that's how cpu_model and threads 
> > > > > > > became
> > > > > > > options for device_add.
> > > > > > > 
> > > > > > > One of the things that David felt was missing from my earlier QMP 
> > > > > > > query
> > > > > > > command (and which is true in your QMP query implementation also) 
> > > > > > > is that
> > > > > > > we aren't exporting cpu_model at all, at least for 
> > > > > > > not-yet-plugged cores.
> > > > > > > So should we include that or let management figure that out since 
> > > > > > > it
> > > > > > > would already know about the CPU model.
> > > > > > 1.
> > > > > > so since you are not planning supporting heterogeneous setup yet,
> > > > > > I'd suggest to refrain from making user to provide cpu_model at
> > > > > > device_add time. Instead make machine code to set it for cores it
> > > > > > creates before core.realize() (yet another use for pre_plug()).
> > > > > > 
> > > > > > That way mgmt doesn't have to figure out what cpu_model to set at
> > > > > > device_add time and doesn't have find out what property to use for 
> > > > > > it.
> > > > > 
> > > > > Yes.. of course you could also do the same thing for nr_threads, so
> > > > > I'm wondering whether there's a good argument to keep one in
> > > > > pre_plug() and one in query-hotpluggable-cpus.
> > > > 
> > > > Right, so what should be the way forward ? Should we keep cpu_model= and
> > > > threads= options with device_add or just threads=  or neither ?
> > > 
> > > I'm inclined to keep them both in device_add - I like the idea of
> > > having an example on day 0 of advertising extra properties (beyond
> > > nr_threads and location) to set from query-hotpluggable-cpus.
> > > 
> > > But, I'd probably change my mind if Igor or someone has a stronger
> > > opinion.  
> > I don't have a strong opinion on this, but you have to keep in mind
> > that one you make it ABI you probably would have to maintain it forever.
> > 
> > So far 'threads' and 'cpu_model' look like a constant values,
> > fixed at start-up time for every core.
> > Taking in account that user is not supposed to change them during
> > hotplug time and that they are the same for every core,
> > I'd go for conservative route and hide them in pre_plug() for now.
> > You always can expose them later if needed.  
> 
> Hm, yes, I see your point.  Hiding them in pre-plug does also make
> life more convenient for someone (not libvirt) manually experimenting
> with this - less to type on their device-add command.
> 
> > > If we advertise cpu_model, however, it should probably be changed to
> > > cpu thread class name, since IIUC that's an existing advertised part
> > > of the QOM interface, but cpu_model isn't.  
> > I still think that spapr-cpu-core should be an abstract type
> > with a concrete set of derived cores types per each thread type.
> > But this question is not related to

Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-21 Thread David Gibson 
On Mon, Mar 21, 2016 at 11:43:34AM +0100, Igor Mammedov wrote:
> On Mon, 21 Mar 2016 14:57:53 +1100
> David Gibson  wrote:
> 
> > On Fri, Mar 18, 2016 at 08:59:32AM +0530, Bharata B Rao wrote:
> > > On Thu, Mar 17, 2016 at 09:03:43PM +1100, David Gibson wrote:  
> > > > On Wed, Mar 16, 2016 at 04:48:50PM +0100, Igor Mammedov wrote:  
> > > > > On Wed, 16 Mar 2016 09:18:03 +0530
> > > > > Bharata B Rao  wrote:
> > > > >   
> > > > > > On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote:  
> > > > > > > On Fri, 11 Mar 2016 10:24:29 +0530
> > > > > > > Bharata B Rao  wrote:
> > > > > > > 
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > This is the next version of "Core based CPU hotplug for PowerPC 
> > > > > > > > sPAPR" that
> > > > > > > > was posted at
> > > > > > > > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> > > > > > > > 
> > > > > > > > device_add semantics
> > > > > > > > 
> > > > > > > > For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> > > > > > > > (qemu) device_add 
> > > > > > > > spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]
> > > > > > > do you plan to allow user to hotplug different cpu_models?
> > > > > > > If not it would be better to hide cpu_model from user
> > > > > > > and set it from machine pre_plug handler.
> > > > > > 
> > > > > > In my earlier implementations I derived cpu model from -cpu and 
> > > > > > threads from
> > > > > > -smp,threads= commandline options and never exposed them to 
> > > > > > device_add
> > > > > > command.
> > > > > > 
> > > > > > Though we don't support heterogenous systems (different cpu models 
> > > > > > and/or
> > > > > > threads) now, it was felt that it should be easy enough to support 
> > > > > > such
> > > > > > systems if required in future, that's how cpu_model and threads 
> > > > > > became
> > > > > > options for device_add.
> > > > > > 
> > > > > > One of the things that David felt was missing from my earlier QMP 
> > > > > > query
> > > > > > command (and which is true in your QMP query implementation also) 
> > > > > > is that
> > > > > > we aren't exporting cpu_model at all, at least for not-yet-plugged 
> > > > > > cores.
> > > > > > So should we include that or let management figure that out since it
> > > > > > would already know about the CPU model.  
> > > > > 1.
> > > > > so since you are not planning supporting heterogeneous setup yet,
> > > > > I'd suggest to refrain from making user to provide cpu_model at
> > > > > device_add time. Instead make machine code to set it for cores it
> > > > > creates before core.realize() (yet another use for pre_plug()).
> > > > > 
> > > > > That way mgmt doesn't have to figure out what cpu_model to set at
> > > > > device_add time and doesn't have find out what property to use for 
> > > > > it.  
> > > > 
> > > > Yes.. of course you could also do the same thing for nr_threads, so
> > > > I'm wondering whether there's a good argument to keep one in
> > > > pre_plug() and one in query-hotpluggable-cpus.  
> > > 
> > > Right, so what should be the way forward ? Should we keep cpu_model= and
> > > threads= options with device_add or just threads=  or neither ?  
> > 
> > I'm inclined to keep them both in device_add - I like the idea of
> > having an example on day 0 of advertising extra properties (beyond
> > nr_threads and location) to set from query-hotpluggable-cpus.
> > 
> > But, I'd probably change my mind if Igor or someone has a stronger
> > opinion.
> I don't have a strong opinion on this, but you have to keep in mind
> that one you make it ABI you probably would have to maintain it forever.
> 
> So far 'threads' and 'cpu_model' look like a constant values,
> fixed at start-up time for every core.
> Taking in account that user is not supposed to change them during
> hotplug time and that they are the same for every core,
> I'd go for conservative route and hide them in pre_plug() for now.
> You always can expose them later if needed.

Hm, yes, I see your point.  Hiding them in pre-plug does also make
life more convenient for someone (not libvirt) manually experimenting
with this - less to type on their device-add command.

> > If we advertise cpu_model, however, it should probably be changed to
> > cpu thread class name, since IIUC that's an existing advertised part
> > of the QOM interface, but cpu_model isn't.
> I still think that spapr-cpu-core should be an abstract type
> with a concrete set of derived cores types per each thread type.
> But this question is not related to hotplug, but rather to
> start-up of QEMU from scratch with -device and supported types
> discovery. So I'd postpone question for later and that's yet another
> reason why I'd like to hide cpu_model from user for now.

Ah.. yes, I think you're probably right, and we should have derived
types.  It's a little bit awkward for spapr, but we can

Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-21 Thread Igor Mammedov 
On Mon, 21 Mar 2016 14:57:53 +1100
David Gibson  wrote:

> On Fri, Mar 18, 2016 at 08:59:32AM +0530, Bharata B Rao wrote:
> > On Thu, Mar 17, 2016 at 09:03:43PM +1100, David Gibson wrote:  
> > > On Wed, Mar 16, 2016 at 04:48:50PM +0100, Igor Mammedov wrote:  
> > > > On Wed, 16 Mar 2016 09:18:03 +0530
> > > > Bharata B Rao  wrote:
> > > >   
> > > > > On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote:  
> > > > > > On Fri, 11 Mar 2016 10:24:29 +0530
> > > > > > Bharata B Rao  wrote:
> > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > This is the next version of "Core based CPU hotplug for PowerPC 
> > > > > > > sPAPR" that
> > > > > > > was posted at
> > > > > > > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> > > > > > > 
> > > > > > > device_add semantics
> > > > > > > 
> > > > > > > For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> > > > > > > (qemu) device_add 
> > > > > > > spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]
> > > > > > do you plan to allow user to hotplug different cpu_models?
> > > > > > If not it would be better to hide cpu_model from user
> > > > > > and set it from machine pre_plug handler.
> > > > > 
> > > > > In my earlier implementations I derived cpu model from -cpu and 
> > > > > threads from
> > > > > -smp,threads= commandline options and never exposed them to device_add
> > > > > command.
> > > > > 
> > > > > Though we don't support heterogenous systems (different cpu models 
> > > > > and/or
> > > > > threads) now, it was felt that it should be easy enough to support 
> > > > > such
> > > > > systems if required in future, that's how cpu_model and threads became
> > > > > options for device_add.
> > > > > 
> > > > > One of the things that David felt was missing from my earlier QMP 
> > > > > query
> > > > > command (and which is true in your QMP query implementation also) is 
> > > > > that
> > > > > we aren't exporting cpu_model at all, at least for not-yet-plugged 
> > > > > cores.
> > > > > So should we include that or let management figure that out since it
> > > > > would already know about the CPU model.  
> > > > 1.
> > > > so since you are not planning supporting heterogeneous setup yet,
> > > > I'd suggest to refrain from making user to provide cpu_model at
> > > > device_add time. Instead make machine code to set it for cores it
> > > > creates before core.realize() (yet another use for pre_plug()).
> > > > 
> > > > That way mgmt doesn't have to figure out what cpu_model to set at
> > > > device_add time and doesn't have find out what property to use for it.  
> > > 
> > > Yes.. of course you could also do the same thing for nr_threads, so
> > > I'm wondering whether there's a good argument to keep one in
> > > pre_plug() and one in query-hotpluggable-cpus.  
> > 
> > Right, so what should be the way forward ? Should we keep cpu_model= and
> > threads= options with device_add or just threads=  or neither ?  
> 
> I'm inclined to keep them both in device_add - I like the idea of
> having an example on day 0 of advertising extra properties (beyond
> nr_threads and location) to set from query-hotpluggable-cpus.
> 
> But, I'd probably change my mind if Igor or someone has a stronger
> opinion.
I don't have a strong opinion on this, but you have to keep in mind
that one you make it ABI you probably would have to maintain it forever.

So far 'threads' and 'cpu_model' look like a constant values,
fixed at start-up time for every core.
Taking in account that user is not supposed to change them during
hotplug time and that they are the same for every core,
I'd go for conservative route and hide them in pre_plug() for now.
You always can expose them later if needed.

> If we advertise cpu_model, however, it should probably be changed to
> cpu thread class name, since IIUC that's an existing advertised part
> of the QOM interface, but cpu_model isn't.
I still think that spapr-cpu-core should be an abstract type
with a concrete set of derived cores types per each thread type.
But this question is not related to hotplug, but rather to
start-up of QEMU from scratch with -device and supported types
discovery. So I'd postpone question for later and that's yet another
reason why I'd like to hide cpu_model from user for now.

Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-20 Thread David Gibson 
On Fri, Mar 18, 2016 at 08:59:32AM +0530, Bharata B Rao wrote:
> On Thu, Mar 17, 2016 at 09:03:43PM +1100, David Gibson wrote:
> > On Wed, Mar 16, 2016 at 04:48:50PM +0100, Igor Mammedov wrote:
> > > On Wed, 16 Mar 2016 09:18:03 +0530
> > > Bharata B Rao  wrote:
> > > 
> > > > On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote:
> > > > > On Fri, 11 Mar 2016 10:24:29 +0530
> > > > > Bharata B Rao  wrote:
> > > > >   
> > > > > > Hi,
> > > > > > 
> > > > > > This is the next version of "Core based CPU hotplug for PowerPC 
> > > > > > sPAPR" that
> > > > > > was posted at
> > > > > > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> > > > > > 
> > > > > > device_add semantics
> > > > > > 
> > > > > > For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> > > > > > (qemu) device_add 
> > > > > > spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]  
> > > > > do you plan to allow user to hotplug different cpu_models?
> > > > > If not it would be better to hide cpu_model from user
> > > > > and set it from machine pre_plug handler.  
> > > > 
> > > > In my earlier implementations I derived cpu model from -cpu and threads 
> > > > from
> > > > -smp,threads= commandline options and never exposed them to device_add
> > > > command.
> > > > 
> > > > Though we don't support heterogenous systems (different cpu models 
> > > > and/or
> > > > threads) now, it was felt that it should be easy enough to support such
> > > > systems if required in future, that's how cpu_model and threads became
> > > > options for device_add.
> > > > 
> > > > One of the things that David felt was missing from my earlier QMP query
> > > > command (and which is true in your QMP query implementation also) is 
> > > > that
> > > > we aren't exporting cpu_model at all, at least for not-yet-plugged 
> > > > cores.
> > > > So should we include that or let management figure that out since it
> > > > would already know about the CPU model.
> > > 1.
> > > so since you are not planning supporting heterogeneous setup yet,
> > > I'd suggest to refrain from making user to provide cpu_model at
> > > device_add time. Instead make machine code to set it for cores it
> > > creates before core.realize() (yet another use for pre_plug()).
> > > 
> > > That way mgmt doesn't have to figure out what cpu_model to set at
> > > device_add time and doesn't have find out what property to use for it.
> > 
> > Yes.. of course you could also do the same thing for nr_threads, so
> > I'm wondering whether there's a good argument to keep one in
> > pre_plug() and one in query-hotpluggable-cpus.
> 
> Right, so what should be the way forward ? Should we keep cpu_model= and
> threads= options with device_add or just threads=  or neither ?

I'm inclined to keep them both in device_add - I like the idea of
having an example on day 0 of advertising extra properties (beyond
nr_threads and location) to set from query-hotpluggable-cpus.

But, I'd probably change my mind if Igor or someone has a stronger
opinion.

If we advertise cpu_model, however, it should probably be changed to
cpu thread class name, since IIUC that's an existing advertised part
of the QOM interface, but cpu_model isn't.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-19 Thread David Gibson 
On Wed, Mar 16, 2016 at 04:48:50PM +0100, Igor Mammedov wrote:
> On Wed, 16 Mar 2016 09:18:03 +0530
> Bharata B Rao  wrote:
> 
> > On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote:
> > > On Fri, 11 Mar 2016 10:24:29 +0530
> > > Bharata B Rao  wrote:
> > >   
> > > > Hi,
> > > > 
> > > > This is the next version of "Core based CPU hotplug for PowerPC sPAPR" 
> > > > that
> > > > was posted at
> > > > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> > > > 
> > > > device_add semantics
> > > > 
> > > > For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> > > > (qemu) device_add 
> > > > spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]  
> > > do you plan to allow user to hotplug different cpu_models?
> > > If not it would be better to hide cpu_model from user
> > > and set it from machine pre_plug handler.  
> > 
> > In my earlier implementations I derived cpu model from -cpu and threads from
> > -smp,threads= commandline options and never exposed them to device_add
> > command.
> > 
> > Though we don't support heterogenous systems (different cpu models and/or
> > threads) now, it was felt that it should be easy enough to support such
> > systems if required in future, that's how cpu_model and threads became
> > options for device_add.
> > 
> > One of the things that David felt was missing from my earlier QMP query
> > command (and which is true in your QMP query implementation also) is that
> > we aren't exporting cpu_model at all, at least for not-yet-plugged cores.
> > So should we include that or let management figure that out since it
> > would already know about the CPU model.
> 1.
> so since you are not planning supporting heterogeneous setup yet,
> I'd suggest to refrain from making user to provide cpu_model at
> device_add time. Instead make machine code to set it for cores it
> creates before core.realize() (yet another use for pre_plug()).
> 
> That way mgmt doesn't have to figure out what cpu_model to set at
> device_add time and doesn't have find out what property to use for it.

Yes.. of course you could also do the same thing for nr_threads, so
I'm wondering whether there's a good argument to keep one in
pre_plug() and one in query-hotpluggable-cpus.

> 2.
> If you still insist on providing cpu_model property at
> device_add time, you'd better extend QMP command query-hotpluggable-cpus
> to provide it in 'props' list along with valid value.

Yes, that's definitely true.

> But I'd go with #1 as questions of using cpu_model-s vs QOM types and
> discovery of mapping of cpu-model to QOM types is not clear yet
> and need to be discussed further.

Hm, I suppose so.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-19 Thread Bharata B Rao 
On Thu, Mar 17, 2016 at 09:03:43PM +1100, David Gibson wrote:
> On Wed, Mar 16, 2016 at 04:48:50PM +0100, Igor Mammedov wrote:
> > On Wed, 16 Mar 2016 09:18:03 +0530
> > Bharata B Rao  wrote:
> > 
> > > On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote:
> > > > On Fri, 11 Mar 2016 10:24:29 +0530
> > > > Bharata B Rao  wrote:
> > > >   
> > > > > Hi,
> > > > > 
> > > > > This is the next version of "Core based CPU hotplug for PowerPC 
> > > > > sPAPR" that
> > > > > was posted at
> > > > > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> > > > > 
> > > > > device_add semantics
> > > > > 
> > > > > For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> > > > > (qemu) device_add 
> > > > > spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]  
> > > > do you plan to allow user to hotplug different cpu_models?
> > > > If not it would be better to hide cpu_model from user
> > > > and set it from machine pre_plug handler.  
> > > 
> > > In my earlier implementations I derived cpu model from -cpu and threads 
> > > from
> > > -smp,threads= commandline options and never exposed them to device_add
> > > command.
> > > 
> > > Though we don't support heterogenous systems (different cpu models and/or
> > > threads) now, it was felt that it should be easy enough to support such
> > > systems if required in future, that's how cpu_model and threads became
> > > options for device_add.
> > > 
> > > One of the things that David felt was missing from my earlier QMP query
> > > command (and which is true in your QMP query implementation also) is that
> > > we aren't exporting cpu_model at all, at least for not-yet-plugged cores.
> > > So should we include that or let management figure that out since it
> > > would already know about the CPU model.
> > 1.
> > so since you are not planning supporting heterogeneous setup yet,
> > I'd suggest to refrain from making user to provide cpu_model at
> > device_add time. Instead make machine code to set it for cores it
> > creates before core.realize() (yet another use for pre_plug()).
> > 
> > That way mgmt doesn't have to figure out what cpu_model to set at
> > device_add time and doesn't have find out what property to use for it.
> 
> Yes.. of course you could also do the same thing for nr_threads, so
> I'm wondering whether there's a good argument to keep one in
> pre_plug() and one in query-hotpluggable-cpus.

Right, so what should be the way forward ? Should we keep cpu_model= and
threads= options with device_add or just threads=  or neither ?

Regards,
Bharata.

Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-18 Thread Igor Mammedov 
On Wed, 16 Mar 2016 09:18:03 +0530
Bharata B Rao  wrote:

> On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote:
> > On Fri, 11 Mar 2016 10:24:29 +0530
> > Bharata B Rao  wrote:
> >   
> > > Hi,
> > > 
> > > This is the next version of "Core based CPU hotplug for PowerPC sPAPR" 
> > > that
> > > was posted at
> > > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> > > 
> > > device_add semantics
> > > 
> > > For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> > > (qemu) device_add 
> > > spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]  
> > do you plan to allow user to hotplug different cpu_models?
> > If not it would be better to hide cpu_model from user
> > and set it from machine pre_plug handler.  
> 
> In my earlier implementations I derived cpu model from -cpu and threads from
> -smp,threads= commandline options and never exposed them to device_add
> command.
> 
> Though we don't support heterogenous systems (different cpu models and/or
> threads) now, it was felt that it should be easy enough to support such
> systems if required in future, that's how cpu_model and threads became
> options for device_add.
> 
> One of the things that David felt was missing from my earlier QMP query
> command (and which is true in your QMP query implementation also) is that
> we aren't exporting cpu_model at all, at least for not-yet-plugged cores.
> So should we include that or let management figure that out since it
> would already know about the CPU model.
1.
so since you are not planning supporting heterogeneous setup yet,
I'd suggest to refrain from making user to provide cpu_model at
device_add time. Instead make machine code to set it for cores it
creates before core.realize() (yet another use for pre_plug()).

That way mgmt doesn't have to figure out what cpu_model to set at
device_add time and doesn't have find out what property to use for it.

2.
If you still insist on providing cpu_model property at
device_add time, you'd better extend QMP command query-hotpluggable-cpus
to provide it in 'props' list along with valid value.

But I'd go with #1 as questions of using cpu_model-s vs QOM types and
discovery of mapping of cpu-model to QOM types is not clear yet
and need to be discussed further.


> 
> Regards,
> Bharata.
> 
>

Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-15 Thread Bharata B Rao 
On Mon, Mar 14, 2016 at 10:47:28AM +0100, Igor Mammedov wrote:
> On Fri, 11 Mar 2016 10:24:29 +0530
> Bharata B Rao  wrote:
> 
> > Hi,
> > 
> > This is the next version of "Core based CPU hotplug for PowerPC sPAPR" that
> > was posted at
> > https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> > 
> > device_add semantics
> > 
> > For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> > (qemu) device_add spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]
> do you plan to allow user to hotplug different cpu_models?
> If not it would be better to hide cpu_model from user
> and set it from machine pre_plug handler.

In my earlier implementations I derived cpu model from -cpu and threads from
-smp,threads= commandline options and never exposed them to device_add
command.

Though we don't support heterogenous systems (different cpu models and/or
threads) now, it was felt that it should be easy enough to support such
systems if required in future, that's how cpu_model and threads became
options for device_add.

One of the things that David felt was missing from my earlier QMP query
command (and which is true in your QMP query implementation also) is that
we aren't exporting cpu_model at all, at least for not-yet-plugged cores.
So should we include that or let management figure that out since it
would already know about the CPU model.

Regards,
Bharata.

Re: [Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-14 Thread Igor Mammedov 
On Fri, 11 Mar 2016 10:24:29 +0530
Bharata B Rao  wrote:

> Hi,
> 
> This is the next version of "Core based CPU hotplug for PowerPC sPAPR" that
> was posted at
> https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html
> 
> device_add semantics
> 
> For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
> (qemu) device_add spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]
do you plan to allow user to hotplug different cpu_models?
If not it would be better to hide cpu_model from user
and set it from machine pre_plug handler.

> 
> Major changes in this version
> -
> - Based on the review feedback, removed the links from machine object
>   to the core objects.
> - With that, the concept of using the links as slots where core object sits
>   is gone.
> - String slot name which was being used as slot= with device_add now
>   becomes an integer core id being specified as core=.
> - threads property which indicates the number of threads in the core
>   moves from spapr-cpu-core type to cpu-core type.
> - Threads creation moves from core's property setter to core's realizefn.
> - Igor's proposed pre_plug handler isn't yet used in this patchset, but it
>   wouldn't take much effort to switch to it. Waiting for some review/consensus
>   on Igor's patchset before switching to it.
> - This patchset will now work with Igor's query-hotpluggable-cpus QMP
>   interface.
> 
> Other changes
> -
> - Core ID that is used with device_add is in fact device tree ID now.
> - DRC indexes are based on core_dt_id now. There are a couple of places
>   where core device's thread0 is used to fetch the DRC index, but changing
>   that requires bigger change of converting the CPUs DT code generation
>   to iterate over cores instead of threads.
> - Coverted while(1) to do-while() as suggeted by Thomas Huth (3/9).
> - Creation of spapr-cpu-core device and conversion of boot CPUs into
>   cores merged into a single patch (6/9).
> - Topologies with incomplete cores are prevented from booting only with
>   machine type versions that support CPU DR (6/9).
> - Conversion of boot CPUs into cores is done only for machine type versions
>   that support CPU DR (6/9).
> - Take care of recovery from failure in plug handler when CPU hotplug isn't
>   supported correctly. This will not be needed when we prevent such
>   attempts from pre_plug handler (9/9).
> Bharata B Rao (8):
>   exec: Remove cpu from cpus list during cpu_exec_exit()
>   exec: Do vmstate unregistration from cpu_exec_exit()
>   cpu: Add a sync version of cpu_remove()
>   cpu: Abstract CPU core type
>   spapr: CPU core device
>   spapr: CPU hotplug support
>   xics,xics_kvm: Handle CPU unplug correctly
>   spapr: CPU hot unplug support
> 
> Gu Zheng (1):
>   cpu: Reclaim vCPU objects
> 
>  cpus.c  |  51 +-
>  exec.c  |  41 -
>  hw/cpu/Makefile.objs|   1 +
>  hw/cpu/core.c   |  87 ++
>  hw/intc/xics.c  |  14 ++
>  hw/intc/xics_kvm.c  |   8 +-
>  hw/ppc/Makefile.objs|   1 +
>  hw/ppc/spapr.c  | 153 +++--
>  hw/ppc/spapr_cpu_core.c | 354 
> 
>  hw/ppc/spapr_events.c   |   3 +
>  hw/ppc/spapr_rtas.c |  24 +++
>  include/hw/cpu/core.h   |  31 
>  include/hw/ppc/spapr.h  |   7 +
>  include/hw/ppc/spapr_cpu_core.h |  42 +
>  include/hw/ppc/xics.h   |   1 +
>  include/qom/cpu.h   |  18 ++
>  include/sysemu/kvm.h|   1 +
>  kvm-all.c   |  57 ++-
>  kvm-stub.c  |   5 +
>  19 files changed, 871 insertions(+), 28 deletions(-)
>  create mode 100644 hw/cpu/core.c
>  create mode 100644 hw/ppc/spapr_cpu_core.c
>  create mode 100644 include/hw/cpu/core.h
>  create mode 100644 include/hw/ppc/spapr_cpu_core.h
>

[Qemu-devel] [RFC PATCH v2 0/9] Core based CPU hotplug for PowerPC sPAPR

2016-03-10 Thread Bharata B Rao 
Hi,

This is the next version of "Core based CPU hotplug for PowerPC sPAPR" that
was posted at
https://lists.gnu.org/archive/html/qemu-ppc/2016-03/msg00081.html

device_add semantics

For -smp 16,sockets=1,cores=2,threads=8,maxcpus=32
(qemu) device_add spapr-cpu-core,id=core2,core=16,cpu_model=host[,threads=8]

Major changes in this version
-
- Based on the review feedback, removed the links from machine object
  to the core objects.
- With that, the concept of using the links as slots where core object sits
  is gone.
- String slot name which was being used as slot= with device_add now
  becomes an integer core id being specified as core=.
- threads property which indicates the number of threads in the core
  moves from spapr-cpu-core type to cpu-core type.
- Threads creation moves from core's property setter to core's realizefn.
- Igor's proposed pre_plug handler isn't yet used in this patchset, but it
  wouldn't take much effort to switch to it. Waiting for some review/consensus
  on Igor's patchset before switching to it.
- This patchset will now work with Igor's query-hotpluggable-cpus QMP
  interface.

Other changes
-
- Core ID that is used with device_add is in fact device tree ID now.
- DRC indexes are based on core_dt_id now. There are a couple of places
  where core device's thread0 is used to fetch the DRC index, but changing
  that requires bigger change of converting the CPUs DT code generation
  to iterate over cores instead of threads.
- Coverted while(1) to do-while() as suggeted by Thomas Huth (3/9).
- Creation of spapr-cpu-core device and conversion of boot CPUs into
  cores merged into a single patch (6/9).
- Topologies with incomplete cores are prevented from booting only with
  machine type versions that support CPU DR (6/9).
- Conversion of boot CPUs into cores is done only for machine type versions
  that support CPU DR (6/9).
- Take care of recovery from failure in plug handler when CPU hotplug isn't
  supported correctly. This will not be needed when we prevent such
  attempts from pre_plug handler (9/9).
Bharata B Rao (8):
  exec: Remove cpu from cpus list during cpu_exec_exit()
  exec: Do vmstate unregistration from cpu_exec_exit()
  cpu: Add a sync version of cpu_remove()
  cpu: Abstract CPU core type
  spapr: CPU core device
  spapr: CPU hotplug support
  xics,xics_kvm: Handle CPU unplug correctly
  spapr: CPU hot unplug support

Gu Zheng (1):
  cpu: Reclaim vCPU objects

 cpus.c  |  51 +-
 exec.c  |  41 -
 hw/cpu/Makefile.objs|   1 +
 hw/cpu/core.c   |  87 ++
 hw/intc/xics.c  |  14 ++
 hw/intc/xics_kvm.c  |   8 +-
 hw/ppc/Makefile.objs|   1 +
 hw/ppc/spapr.c  | 153 +++--
 hw/ppc/spapr_cpu_core.c | 354 
 hw/ppc/spapr_events.c   |   3 +
 hw/ppc/spapr_rtas.c |  24 +++
 include/hw/cpu/core.h   |  31 
 include/hw/ppc/spapr.h  |   7 +
 include/hw/ppc/spapr_cpu_core.h |  42 +
 include/hw/ppc/xics.h   |   1 +
 include/qom/cpu.h   |  18 ++
 include/sysemu/kvm.h|   1 +
 kvm-all.c   |  57 ++-
 kvm-stub.c  |   5 +
 19 files changed, 871 insertions(+), 28 deletions(-)
 create mode 100644 hw/cpu/core.c
 create mode 100644 hw/ppc/spapr_cpu_core.c
 create mode 100644 include/hw/cpu/core.h
 create mode 100644 include/hw/ppc/spapr_cpu_core.h

-- 
2.1.0