diff mbox

[RFC,06/16] vl: move smp parsing to machine pre_init

Message ID 1465580427-13596-7-git-send-email-drjones@redhat.com
State New
Headers show

Commit Message

Andrew Jones June 10, 2016, 5:40 p.m. UTC
Move the guts of smp_parse() into hw/core/machine.c to operate on
smp machine properties, and to eventually allow it to be overridden
by machines. We leave the smp_parse function behind to handle the
(now deprecated) -smp option, but now it only needs to set the
machine properties.

Signed-off-by: Andrew Jones <drjones@redhat.com>

---
 hw/core/machine.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 vl.c              | 111 ++++++++++++++++-------------------------------------
 2 files changed, 142 insertions(+), 82 deletions(-)

-- 
2.4.11

Comments

Andrew Jones June 13, 2016, 8:35 p.m. UTC | #1
On Mon, Jun 13, 2016 at 07:04:01PM +0200, Paolo Bonzini wrote:
> 

> 

> On 10/06/2016 19:40, Andrew Jones wrote:

> > +    if (sockets == -1 || cores == -1 || threads == -1 ||

> > +        maxcpus == -1 || cpus == -1) {

> > +        error_report("cpu topology: "

> > +                     "all machine properties must be specified");

> > +        exit(1);

> > +    }

> > +

> 

> I think it's sane to accept some defaults.  It must not be the DWIM


I agree requiring all these properties means a lot of typing, but I'm
not sure we want to calculate defaults. Reasoning below

> thing that -smp does (which is targeted to Windows's dislike of

> multi-socket machine on consumer hardware).  It must be something that

> makes sense, and my proposal is:

> 

> - threads: 1

> - cores: 1

> - sockets:

>   - maxcpus / (cores * threads) if maxcpus given

>   - cpus / (cores * threads) if cpus given

>   - else 1

> - maxcpus: cores * threads * sockets

> - cpus: maxcpus


I think some machines may prefer

- threads: 1
- sockets: 1
- cores:
  - maxcpus / (sockets * threads) if maxcpus given
  - cpus / (sockets * threads) if cpus given
  - else 1

> 

> This is a simple, linear logic that assigns defaults to each argument in

> sequence.  At the same time -machine should fail for all invalid

> combinations, namely:

> 

> - any argument < 1


What if a user says threads=0, because they don't have HT, and assume
that a value of zero will get them a non-HT system? Or what about
machines that don't describe sockets, so a user inputs zero? In both
those cases I agree we should just use 1, but from a user's perspective,
it might seem weird.

> - any argument > some compile-time value (1024?) to avoid overflows


Agreed. We should do this regardless of this series.

> - cpus % (cores * threads) != 0


Hmm. This makes sense where cpus is the number of cpu packages,
but I guess that's not how things currently work. Maybe Igor's
work is changing that?

> - cpus > sockets * cores * threads

> - maxcpus != cores * threads * sockets


We check these two (the 2nd added with this series) already.

> 

> Alone the last relation shows that requiring all four of maxcpus, cores,

> threads and sockets is unnecessary. :)


Not really. It depends on if you assume sockets, cores or threads to
be the N in maxcpus=N. We could just require sockets, cores, and threads
to be input, which allows us to easily calculate maxcpus, and then set
cpus from that. In that case maxcpus would just be an available input
for sanity checking.

> 

> This will catch situations where assigning a default for sockets doesn't

> work well.  For example cores=2,cpus=4,maxcpus=5 will assign

> threads=1,sockets=2; then the last relation will fail and QEMU will give

> an error.

> 

> These invalid combination also make sure that other useful default cases

> work.  For example "maxcpus given, sockets and cpus not" works thanks to

> the following deduction:

> 

> - sockets = cpus/(cores*threads)

> - maxcpus = cores*threads*sockets

> - because cpus%(cores*threads) must be 0, this gives maxcpus=cpus

> 

> -smp should do its legacy magic and assign all five values based on it.

> If the results do not match the obvious s/smp/-machine/ command line it

> should warn, and perhaps suggest the equivalent -machine command line.

> It doesn't have to be a minimal command line, just equivalent.


This is a good idea. I'm just still not sure what the -machine command
line should/should not assume. I'm still leaning towards nothing, but
at least maxcpus and cpus could be optional, making it just 3 required
inputs. And, machines are free to override this, allowing -machine
MACHINE,cpus=N to generate their default topology supporting N cpus,
maxcpus also =N. Or, -machine MACHINE,maxcpus=N,cpus=M for their common
hotplug-enabling case.

> 

> Apart from this point, these are lovely patches. :)


Thanks!

drew
Andrew Jones June 14, 2016, 11:39 a.m. UTC | #2
On Tue, Jun 14, 2016 at 10:17:49AM +0200, Paolo Bonzini wrote:
> 

> 

> On 13/06/2016 22:35, Andrew Jones wrote:

> > On Mon, Jun 13, 2016 at 07:04:01PM +0200, Paolo Bonzini wrote:

> >> On 10/06/2016 19:40, Andrew Jones wrote:

> >>> +    if (sockets == -1 || cores == -1 || threads == -1 ||

> >>> +        maxcpus == -1 || cpus == -1) {

> >>> +        error_report("cpu topology: "

> >>> +                     "all machine properties must be specified");

> >>> +        exit(1);

> >>> +    }

> >>> +

> >>

> >> I think it's sane to accept some defaults.  It must not be the DWIM

> >> thing that -smp does (which is targeted to Windows's dislike of

> >> multi-socket machine on consumer hardware).  It must be something that

> >> makes sense, and my proposal is:

> >>

> >> - threads: 1

> >> - cores: 1

> >> - sockets:

> >>   - maxcpus / (cores * threads) if maxcpus given

> >>   - cpus / (cores * threads) if cpus given

> >>   - else 1

> >> - maxcpus: cores * threads * sockets

> >> - cpus: maxcpus

> > 

> > I think some machines may prefer

> > 

> > - threads: 1

> > - sockets: 1

> > - cores:

> >   - maxcpus / (sockets * threads) if maxcpus given

> >   - cpus / (sockets * threads) if cpus given

> >   - else 1

> 

> smp_cores is only used by pseries and x86 machines.  I expect machines

> that must be single-socket to disregard smp_sockets altogether.


mach-virt currently assumes 1 "socket" with N cores when -smp N is used.
This is because, until recently, ARM machines didn't define sockets. I
presume the mindset will remain that cpus=N means cores=N even after
introducing support for multi-socket topology.

> 

> >> - any argument < 1

> > 

> > What if a user says threads=0, because they don't have HT, and assume

> > that a value of zero will get them a non-HT system? Or what about

> > machines that don't describe sockets, so a user inputs zero? In both

> > those cases I agree we should just use 1, but from a user's perspective,

> > it might seem weird.

> 

> They should just not specify it and get a default of 1. ;)


Yeah, threads, the only one we should never calculate, could be
optional. If not specified, defaulting to 1 makes perfect sense.
But, threads=0, which is weird, but in a way specifying that it's
not specified, also makes some sense.

> 

> >> - any argument > some compile-time value (1024?) to avoid overflows

> > 

> > Agreed. We should do this regardless of this series.

> > 

> >> - cpus % (cores * threads) != 0

> > 

> > Hmm. This makes sense where cpus is the number of cpu packages,

> 

> I'm not sure I understand what you mean here.  The point is that the

> machine starts with an integral number of sockets.


OK, s/cpus/maxcpus/ then. By using the currently online number, I
thought you were starting to prepare for cpu packages, which are
indivisible sets of cores and threads. But now that I think about
it, cpus % (cores * threads) isn't right for that either. It should
be total-online-threads % (cores * threads) != 0

> 

> >> - cpus > sockets * cores * threads

> >> - maxcpus != cores * threads * sockets

> > 

> > We check these two (the 2nd added with this series) already.

> 

> Yup, I was just making a complete list.

> 

> >> Alone the last relation shows that requiring all four of maxcpus, cores,

> >> threads and sockets is unnecessary. :)

> > 

> > Not really. It depends on if you assume sockets, cores or threads to

> > be the N in maxcpus=N. We could just require sockets, cores, and threads

> > to be input, which allows us to easily calculate maxcpus, and then set

> > cpus from that. In that case maxcpus would just be an available input

> > for sanity checking.

> 

> Or you could just specify -machine cpus=16,maxcpus=32 and expect 1

> core/socket, 1 thread/core, and 16 to 32 sockets.


Or 32 cores/socket (only 16 populated), 1 thread/core

> 

> >> -smp should do its legacy magic and assign all five values based on it.

> >> If the results do not match the obvious s/smp/-machine/ command line it

> >> should warn, and perhaps suggest the equivalent -machine command line.

> >> It doesn't have to be a minimal command line, just equivalent.

> > 

> > This is a good idea. I'm just still not sure what the -machine command

> > line should/should not assume.

> 

> It's okay.  Adding defaults can be done later, as long as strict checks

> are in place from the beginning and there is a clean separation between

> -smp defaults and -machine.

> 

> Relaxing checks can be done later, so I am much more interested in

> having strict checks than in having defaults.


Yes, my current thinking (which is always adjustable :-) is to start
with strict checks, and maybe always have them in this common code.
Machines that override this can implement defaults based on their
machine-specific knowledge, allowing the user to get sane topologies
without really needing to know what they want.

> 

> Though I think that we can at least agree on defaults for threads,

> maxcpus and cpus.  The only sticky point is sockets vs. cores.  We can

> make them both mandatory for now.  That is: cores and sockets are

> mandatory until we decide which one to default to 1; threads is

> optional; cpus is mandatory if you want hotplug, otherwise it's

> redundant; maxcpus is optional and always redundant.


Agreed. I'll do the following for the next round of this series

 threads: 1
 cores:   required
 sockets: required
 maxcpus: maxcpus ? maxcpus : sockets * cores * threads
 cpus:    cpus ? cpus : maxcpus

If maxcpus is input, it's redundant, but should be sanity checked.
Maybe the user wants to use the QEMU cmdline to check their math...

Thanks,
drew
Andrew Jones June 14, 2016, 2:03 p.m. UTC | #3
On Tue, Jun 14, 2016 at 01:53:05PM +0200, Paolo Bonzini wrote:
> 

> 

> On 14/06/2016 13:39, Andrew Jones wrote:

> > On Tue, Jun 14, 2016 at 10:17:49AM +0200, Paolo Bonzini wrote:

> >> On 13/06/2016 22:35, Andrew Jones wrote:

> >>> On Mon, Jun 13, 2016 at 07:04:01PM +0200, Paolo Bonzini wrote:

> >>>> On 10/06/2016 19:40, Andrew Jones wrote:

> >> They should just not specify it and get a default of 1. ;)

> > 

> > Yeah, threads, the only one we should never calculate, could be

> > optional. If not specified, defaulting to 1 makes perfect sense.

> > But, threads=0, which is weird, but in a way specifying that it's

> > not specified, also makes some sense.

> 

> If it's weird, let's make it invalid. :)


I'm weird, and starting to like it :-)

> 

> >>>> - cpus % (cores * threads) != 0

> >>>

> >>> Hmm. This makes sense where cpus is the number of cpu packages,

> >>

> >> I'm not sure I understand what you mean here.  The point is that the

> >> machine starts with an integral number of sockets.

> > 

> > OK, s/cpus/maxcpus/ then. By using the currently online number, I

> > thought you were starting to prepare for cpu packages, which are

> > indivisible sets of cores and threads.

> 

> Yes, that's what I meant to do.  Isn't cpus what you call

> "total-online-threads" below?


It is. I was thinking it may need to be redefined if we wanted to
hotplug these "indivisible sets of cores and threads" (sockets),
instead of what we do today, which is to hotplug one "cpu" at a time.
However, I just chatted with Igor, and he says cpu hotplug operates
on logical processors, and thus it's fine to talk about hotplugging
threads. Even real hardware does this. Real hardware will plug a
socket full of threads, but then the firmware may keep most of them
disabled until they're "hotplugged". So, I think the value of cpus
can be anything. Even the useless value of zero.

> 

> > But now that I think about

> > it, cpus % (cores * threads) isn't right for that either. It should

> > be total-online-threads % (cores * threads) != 0

> > 

> >> [...] you could just specify -machine cpus=16,maxcpus=32 and expect 1

> >> core/socket, 1 thread/core, and 16 to 32 sockets.

> > 

> > Or 32 cores/socket (only 16 populated), 1 thread/core

> 

> Does that make sense?  How do you "populate" parts of a socket lazily? I

> know it's all virtual, but... ;)


In real hardware that would be a socket of 32 cores, 16 disabled.
Hotplug can then enable them one at a time.

> 

> >> Though I think that we can at least agree on defaults for threads,

> >> maxcpus and cpus.  The only sticky point is sockets vs. cores.  We can

> >> make them both mandatory for now.  That is: cores and sockets are

> >> mandatory until we decide which one to default to 1; threads is

> >> optional; cpus is mandatory if you want hotplug, otherwise it's

> >> redundant; maxcpus is optional and always redundant.

> > 

> > Agreed. I'll do the following for the next round of this series

> > 

> >  threads: 1

> >  cores:   required

> >  sockets: required

> >  maxcpus: maxcpus ? maxcpus : sockets * cores * threads

> >  cpus:    cpus ? cpus : maxcpus

> > 

> > If maxcpus is input, it's redundant, but should be sanity checked.

> > Maybe the user wants to use the QEMU cmdline to check their math...

> 

> Yes, all this is fine.  As to the checks, here's my list:

> 

> >>> - any argument < 1

> >>> - any argument > some compile-time value (1024?) to avoid overflows

> >>> - cpus % (cores * threads) != 0

> >>> - cpus > sockets * cores * threads

> >>> - maxcpus != cores * threads * sockets

> 

> ?  The second, fourth and fifth are fine, and IMO the first too.  What

> about the "integral package" check?   It needs to be disabled with some

> ugly global when using -smp, but apart from that I believe it's better

> to have it.


I'd keep =0 allowed. In most cases it just means =1, same as if the
property wasn't input. In some cases, cpus,maxcpus it creates a
useless machine, but maybe could accelerate QEMU runs for the purpose
of probing? Eh, it probably adds more complication than necessary...

Considering cpus should mean logical processors, I think that can
have any value on (0, maxcpus]

Thanks,
drew
Andrew Jones June 15, 2016, 7:19 a.m. UTC | #4
On Wed, Jun 15, 2016 at 10:51:08AM +1000, David Gibson wrote:
> On Tue, Jun 14, 2016 at 04:03:29PM +0200, Andrew Jones wrote:

> > On Tue, Jun 14, 2016 at 01:53:05PM +0200, Paolo Bonzini wrote:

> > > 

> > > 

> > > On 14/06/2016 13:39, Andrew Jones wrote:

> > > > On Tue, Jun 14, 2016 at 10:17:49AM +0200, Paolo Bonzini wrote:

> > > >> On 13/06/2016 22:35, Andrew Jones wrote:

> > > >>> On Mon, Jun 13, 2016 at 07:04:01PM +0200, Paolo Bonzini wrote:

> > > >>>> On 10/06/2016 19:40, Andrew Jones wrote:

> > > >> They should just not specify it and get a default of 1. ;)

> > > > 

> > > > Yeah, threads, the only one we should never calculate, could be

> > > > optional. If not specified, defaulting to 1 makes perfect sense.

> > > > But, threads=0, which is weird, but in a way specifying that it's

> > > > not specified, also makes some sense.

> > > 

> > > If it's weird, let's make it invalid. :)

> > 

> > I'm weird, and starting to like it :-)

> > 

> > > 

> > > >>>> - cpus % (cores * threads) != 0

> > > >>>

> > > >>> Hmm. This makes sense where cpus is the number of cpu packages,

> > > >>

> > > >> I'm not sure I understand what you mean here.  The point is that the

> > > >> machine starts with an integral number of sockets.

> > > > 

> > > > OK, s/cpus/maxcpus/ then. By using the currently online number, I

> > > > thought you were starting to prepare for cpu packages, which are

> > > > indivisible sets of cores and threads.

> > > 

> > > Yes, that's what I meant to do.  Isn't cpus what you call

> > > "total-online-threads" below?

> > 

> > It is. I was thinking it may need to be redefined if we wanted to

> > hotplug these "indivisible sets of cores and threads" (sockets),

> > instead of what we do today, which is to hotplug one "cpu" at a time.

> > However, I just chatted with Igor, and he says cpu hotplug operates

> > on logical processors, and thus it's fine to talk about hotplugging

> > threads. Even real hardware does this. Real hardware will plug a

> > socket full of threads, but then the firmware may keep most of them

> > disabled until they're "hotplugged". So, I think the value of cpus

> > can be anything. Even the useless value of zero.

> 

> Uh.. this depends on the platform (machine type).  ACPI allows

> (logical) hotplugging of individual threads.  However on pseries the

> hotplug mechanism works at core granularity only.  I don't know of

> real examples off hand, but it's easy to imagine a platform interface

> which worked at socket, or even a multi-socket-module level

> 

> In other words the "cpu package" doesn't necessarily lie at a fixed

> level of the thread/core/socket heirarchy.

> 

> This is something we're grappling with in trying to implement

> cross-platform cpu hotplug, and we obviously don't want to make it

> worse with these -smp changes.


Right now we're bringing hotplug up in this discussion purely to set
the largest range of inputs for each of the five properties, in
order to do a first pass of sanity checks. If a particular machine
has a stricter range for a given property then it's free to override
pre-init and do what it likes. If pre-init gets used for non-smp
related stuff in the future then we'll make a specific smp-init machine
method, allowing smp init to always be easily overridden by machines.

Thanks,
drew
diff mbox

Patch

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2625044002e57..75c5a1fdd7de1 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -17,6 +17,7 @@ 
 #include "qapi/visitor.h"
 #include "hw/sysbus.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/replay.h"
 #include "qemu/error-report.h"
 #include "qemu/cutils.h"
 
@@ -417,16 +418,122 @@  static void machine_init_notify(Notifier *notifier, void *data)
 
 static void machine_set_smp_parameters(MachineState *ms)
 {
-    if (ms->sockets != -1 || ms->cores != -1 || ms->threads != -1 ||
-        ms->maxcpus != -1 || ms->cpus != -1) {
+    int sockets = ms->sockets;
+    int cores   = ms->cores;
+    int threads = ms->threads;
+    int maxcpus = ms->maxcpus;
+    int cpus    = ms->cpus;
+    bool sockets_input = sockets > 0;
+
+    if (sockets == -1 && cores == -1 && threads == -1 &&
+        maxcpus == -1 && cpus == -1) {
+        ms->sockets = 1;
+        ms->cores   = 1;
+        ms->threads = 1;
+        ms->maxcpus = 1;
+        ms->cpus    = 1;
+        return;
+    }
+
+    if (sockets == -1 || cores == -1 || threads == -1 ||
+        maxcpus == -1 || cpus == -1) {
+        error_report("cpu topology: "
+                     "all machine properties must be specified");
+        exit(1);
+    }
+
+    /* If the deprecated -smp option was used without complete input,
+     * or a user input zeros (why would they do that?), then we compute
+     * missing values, preferring sockets over cores over threads.
+     */
+    if (cpus == 0 || sockets == 0) {
+        sockets = sockets > 0 ? sockets : 1;
+        cores = cores > 0 ? cores : 1;
+        threads = threads > 0 ? threads : 1;
+        if (cpus == 0) {
+            cpus = cores * threads * sockets;
+        }
+    } else if (cores == 0) {
+        threads = threads > 0 ? threads : 1;
+        cores = cpus / (sockets * threads);
+    } else if (threads == 0) {
+        threads = cpus / (cores * sockets);
+    } else if (sockets * cores * threads < cpus) {
+        error_report("cpu topology: "
+                     "sockets (%u) * cores (%u) * threads (%u) < "
+                     "smp_cpus (%u)",
+                     sockets, cores, threads, cpus);
+        exit(1);
+    }
+
+    maxcpus = maxcpus > 0 ? maxcpus : cpus;
+
+    if (maxcpus > MAX_CPUMASK_BITS) {
+        error_report("unsupported number of maxcpus");
+        exit(1);
+    }
+
+    if (maxcpus < cpus) {
+        error_report("maxcpus must be equal to or greater than smp");
+        exit(1);
+    }
+
+    if (sockets * cores * threads > maxcpus) {
+        error_report("cpu topology: "
+                     "sockets (%u) * cores (%u) * threads (%u) > "
+                     "maxcpus (%u)",
+                     sockets, cores, threads, maxcpus);
+        exit(1);
+    }
+
+    if (sockets_input && sockets * cores * threads != maxcpus) {
+        unsigned sockets_rounded = DIV_ROUND_UP(maxcpus, cores * threads);
+
         error_report("warning: cpu topology: "
-                     "machine properties currently ignored");
+                     "sockets (%u) * cores (%u) * threads (%u) != "
+                     "maxcpus (%u). Trying sockets=%u.",
+                     sockets, cores, threads, maxcpus, sockets_rounded);
+        sockets = sockets_rounded;
+
+        if (sockets * cores * threads > maxcpus) {
+            error_report("cpu topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) > "
+                         "maxcpus (%u)",
+                         sockets, cores, threads, maxcpus);
+            exit(1);
+        }
     }
+
+    ms->sockets = sockets;
+    ms->cores   = cores;
+    ms->threads = threads;
+    ms->maxcpus = maxcpus;
+    ms->cpus    = cpus;
 }
 
 static void machine_pre_init(MachineState *ms)
 {
+    MachineClass *mc = MACHINE_CLASS(object_get_class(OBJECT(ms)));
+
     machine_set_smp_parameters(ms);
+    smp_cores   = ms->cores;
+    smp_threads = ms->threads;
+    max_cpus    = ms->maxcpus;
+    smp_cpus    = ms->cpus;
+
+    mc->max_cpus = mc->max_cpus ?: 1; /* Default to UP */
+    if (ms->maxcpus > mc->max_cpus) {
+        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
+                     "supported by machine '%s' (%d)", ms->maxcpus, mc->name,
+                     mc->max_cpus);
+        exit(1);
+    }
+
+    if (ms->cpus > 1) {
+        Error *blocker = NULL;
+        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+        replay_add_blocker(blocker);
+    }
 }
 
 static void machine_class_init(ObjectClass *oc, void *data)
diff --git a/vl.c b/vl.c
index 4849dd465d667..843b7a9dff753 100644
--- a/vl.c
+++ b/vl.c
@@ -1222,81 +1222,41 @@  static QemuOptsList qemu_smp_opts = {
 
 static void smp_parse(QemuOpts *opts)
 {
-    if (opts) {
-        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
-        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
-        unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
-        unsigned threads = qemu_opt_get_number(opts, "threads", 0);
-        bool sockets_input = sockets > 0;
-
-        /* compute missing values, prefer sockets over cores over threads */
-        if (cpus == 0 || sockets == 0) {
-            sockets = sockets > 0 ? sockets : 1;
-            cores = cores > 0 ? cores : 1;
-            threads = threads > 0 ? threads : 1;
-            if (cpus == 0) {
-                cpus = cores * threads * sockets;
-            }
-        } else if (cores == 0) {
-            threads = threads > 0 ? threads : 1;
-            cores = cpus / (sockets * threads);
-        } else if (threads == 0) {
-            threads = cpus / (cores * sockets);
-        } else if (sockets * cores * threads < cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) < "
-                         "smp_cpus (%u)",
-                         sockets, cores, threads, cpus);
-            exit(1);
-        }
-
-        max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
-
-        if (max_cpus > MAX_CPUMASK_BITS) {
-            error_report("unsupported number of maxcpus");
-            exit(1);
-        }
-
-        if (max_cpus < cpus) {
-            error_report("maxcpus must be equal to or greater than smp");
-            exit(1);
-        }
+    Object *machine_obj = OBJECT(current_machine);
+    int sockets, cores, threads, maxcpus, cpus;
 
-        if (sockets * cores * threads > max_cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) > "
-                         "maxcpus (%u)",
-                         sockets, cores, threads, max_cpus);
-            exit(1);
-        }
+    if (!opts) {
+        return;
+    }
 
-        if (sockets_input && sockets * cores * threads != max_cpus) {
-            unsigned sockets_rounded = DIV_ROUND_UP(max_cpus, cores * threads);
+    sockets = object_property_get_int(machine_obj, "sockets", NULL);
+    cores   = object_property_get_int(machine_obj, "cores",   NULL);
+    threads = object_property_get_int(machine_obj, "threads", NULL);
+    maxcpus = object_property_get_int(machine_obj, "maxcpus", NULL);
+    cpus    = object_property_get_int(machine_obj, "cpus",    NULL);
 
-            error_report("warning: cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) != "
-                         "maxcpus (%u). Trying sockets=%u.",
-                         sockets, cores, threads, max_cpus, sockets_rounded);
-            sockets = sockets_rounded;
+    if (sockets == -1 && cores == -1 && threads == -1 &&
+        maxcpus == -1 && cpus == -1) {
 
-            if (sockets * cores * threads > max_cpus) {
-                error_report("cpu topology: "
-                             "sockets (%u) * cores (%u) * threads (%u) > "
-                             "maxcpus (%u)",
-                             sockets, cores, threads, max_cpus);
-                exit(1);
-            }
-        }
+        error_report("warning: cpu topology: "
+                     "using deprecated -smp option. "
+                     "Use machine properties instead");
 
-        smp_cpus = cpus;
-        smp_cores = cores;
-        smp_threads = threads;
-    }
+        cpus    = qemu_opt_get_number(opts, "cpus",    0);
+        sockets = qemu_opt_get_number(opts, "sockets", 0);
+        cores   = qemu_opt_get_number(opts, "cores",   0);
+        threads = qemu_opt_get_number(opts, "threads", 0);
+        maxcpus = qemu_opt_get_number(opts, "maxcpus", cpus);
 
-    if (smp_cpus > 1) {
-        Error *blocker = NULL;
-        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
-        replay_add_blocker(blocker);
+        object_property_set_int(machine_obj, sockets, "sockets", NULL);
+        object_property_set_int(machine_obj, cores,   "cores",   NULL);
+        object_property_set_int(machine_obj, threads, "threads", NULL);
+        object_property_set_int(machine_obj, maxcpus, "maxcpus", NULL);
+        object_property_set_int(machine_obj, cpus,    "cpus",    NULL);
+    } else {
+        error_report("warning: cpu topology: "
+                     "both machine properties and -smp option provided. "
+                     "Ignoring the deprecated -smp option");
     }
 }
 
@@ -4106,16 +4066,6 @@  int main(int argc, char **argv, char **envp)
         data_dir[data_dir_idx++] = CONFIG_QEMU_DATADIR;
     }
 
-    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
-
-    machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
-    if (max_cpus > machine_class->max_cpus) {
-        error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
-                     "supported by machine '%s' (%d)", max_cpus,
-                     machine_class->name, machine_class->max_cpus);
-        exit(1);
-    }
-
     /*
      * Get the default machine options from the machine if it is not already
      * specified either by the configuration file or by the command line.
@@ -4308,6 +4258,9 @@  int main(int argc, char **argv, char **envp)
         qtest_init(qtest_chrdev, qtest_log, &error_fatal);
     }
 
+    /* smp_parse must come after qemu_opt_foreach(machine_opts, ...) */
+    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
+
     machine_opts = qemu_get_machine_opts();
     kernel_filename = qemu_opt_get(machine_opts, "kernel");
     initrd_filename = qemu_opt_get(machine_opts, "initrd");