| Message ID | 1626281596-31061-2-git-send-email-pmorel@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: CPU Topology | expand |
On Wed, Jul 14 2021, Pierre Morel <pmorel@linux.ibm.com> wrote: > We need a s390x dedicated SMP parsing to handle s390x specificities. > > In this patch we only handle threads, cores and sockets for > s390x: > - do not support threads, we always have 1 single thread per core > - the sockets are filled one after the other with the cores > > Both these handlings are different from the standard smp_parse > functionement and reflect the CPU topology in the simple case > where all CPU belong to the same book. > > Topology levels above sockets, i.e. books, drawers, are not > considered at this stage and will be introduced in a later patch. > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 42 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index e4b18aef49..899d3a4137 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -582,6 +582,47 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz) > return newsz; > } > > +/* > + * In S390CCW machine we do not support threads for now, > + * only sockets and cores. > + */ > +static void s390_smp_parse(MachineState *ms, QemuOpts *opts) It seems you based this on an older version of the code? The current signature of this function since 1e63fe685804 ("machine: pass QAPI struct to mc->smp_parse") is void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp); That affects your parsing, and also lets you get rid of the ugly exit(1) statements. > +{ > + unsigned cpus = qemu_opt_get_number(opts, "cpus", 1); > + unsigned sockets = qemu_opt_get_number(opts, "sockets", 1); > + unsigned cores = qemu_opt_get_number(opts, "cores", 1); > + > + if (opts) { > + if (cpus == 0 || sockets == 0 || cores == 0) { This behaviour looks different from what we do for other targets: if you specify the value as 0, a value is calculated from the other values; here, you error out. It's probably not a good idea to differ. > + error_report("cpu topology: " > + "sockets (%u), cores (%u) or number of CPU(%u) " > + "can not be zero", sockets, cores, cpus); > + exit(1); > + } > + } > +
On Fri, Jul 16, 2021 at 10:54:08AM +0200, Cornelia Huck wrote: > On Wed, Jul 14 2021, Pierre Morel <pmorel@linux.ibm.com> wrote: > > > We need a s390x dedicated SMP parsing to handle s390x specificities. > > > > In this patch we only handle threads, cores and sockets for > > s390x: > > - do not support threads, we always have 1 single thread per core > > - the sockets are filled one after the other with the cores > > > > Both these handlings are different from the standard smp_parse > > functionement and reflect the CPU topology in the simple case > > where all CPU belong to the same book. > > > > Topology levels above sockets, i.e. books, drawers, are not > > considered at this stage and will be introduced in a later patch. > > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > > --- > > hw/s390x/s390-virtio-ccw.c | 42 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 42 insertions(+) > > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > > index e4b18aef49..899d3a4137 100644 > > --- a/hw/s390x/s390-virtio-ccw.c > > +++ b/hw/s390x/s390-virtio-ccw.c > > @@ -582,6 +582,47 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz) > > return newsz; > > } > > > > +/* > > + * In S390CCW machine we do not support threads for now, > > + * only sockets and cores. > > + */ > > +static void s390_smp_parse(MachineState *ms, QemuOpts *opts) > > It seems you based this on an older version of the code? The current > signature of this function since 1e63fe685804 ("machine: pass QAPI > struct to mc->smp_parse") is > > void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp); > > That affects your parsing, and also lets you get rid of the ugly exit(1) > statements. > > > +{ > > + unsigned cpus = qemu_opt_get_number(opts, "cpus", 1); > > + unsigned sockets = qemu_opt_get_number(opts, "sockets", 1); > > + unsigned cores = qemu_opt_get_number(opts, "cores", 1); > > + > > + if (opts) { > > + if (cpus == 0 || sockets == 0 || cores == 0) { > > This behaviour looks different from what we do for other targets: if you > specify the value as 0, a value is calculated from the other values; > here, you error out. It's probably not a good idea to differ. I increasingly worry that we're making a mistake by going down the route of having custom smp_parse implementations per target, as this is showing signs of inconsistent behaviour and error reportings. I think the differences / restrictions have granularity at a different level that is being tested in many cases too. Whether threads != 1 is valid will likely vary depending on what CPU model is chosen, rather than what architecture is chosen. The same is true for dies != 1. We're not really checking this closely even in x86 - for example I can request nonsense such as a 25 year old i486 CPU model with hyperthreading and multiple dies qemu-system-x86_64 -cpu 486 -smp 16,cores=4,dies=2,threads=2 In this patch, there is no error reporting if the user specifies dies != 1 or threads != 1 - it just silently ignores the request which is not good. Some machine types may have constraints on CPU sockets. This can of course all be handled by custom smp_parse impls, but this is ultimately going to lead to alot of duplicated and inconsistent logic I fear. I wonder if we would be better off having machine class callback that can report topology constraints for the current configuration, along lines of smp_constraints(MachineState *ms, int *max_sockets, int *max_dies, int *max_cores, int *max_threads) And then have only a single smp_parse impl that takes into account these constraints, to report errors / fill in missing fields / etc ? Regards, Daniel
On 7/16/21 10:54 AM, Cornelia Huck wrote: > On Wed, Jul 14 2021, Pierre Morel <pmorel@linux.ibm.com> wrote: > >> We need a s390x dedicated SMP parsing to handle s390x specificities. >> >> In this patch we only handle threads, cores and sockets for >> s390x: >> - do not support threads, we always have 1 single thread per core >> - the sockets are filled one after the other with the cores >> >> Both these handlings are different from the standard smp_parse >> functionement and reflect the CPU topology in the simple case >> where all CPU belong to the same book. >> >> Topology levels above sockets, i.e. books, drawers, are not >> considered at this stage and will be introduced in a later patch. >> >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> hw/s390x/s390-virtio-ccw.c | 42 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 42 insertions(+) >> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index e4b18aef49..899d3a4137 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -582,6 +582,47 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz) >> return newsz; >> } >> >> +/* >> + * In S390CCW machine we do not support threads for now, >> + * only sockets and cores. >> + */ >> +static void s390_smp_parse(MachineState *ms, QemuOpts *opts) > > It seems you based this on an older version of the code? The current > signature of this function since 1e63fe685804 ("machine: pass QAPI > struct to mc->smp_parse") is > > void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp); > > That affects your parsing, and also lets you get rid of the ugly exit(1) > statements. hum, yes, thanks > >> +{ >> + unsigned cpus = qemu_opt_get_number(opts, "cpus", 1); >> + unsigned sockets = qemu_opt_get_number(opts, "sockets", 1); >> + unsigned cores = qemu_opt_get_number(opts, "cores", 1); >> + >> + if (opts) { >> + if (cpus == 0 || sockets == 0 || cores == 0) { > > This behaviour looks different from what we do for other targets: if you > specify the value as 0, a value is calculated from the other values; > here, you error out. It's probably not a good idea to differ. right, thanks > >> + error_report("cpu topology: " >> + "sockets (%u), cores (%u) or number of CPU(%u) " >> + "can not be zero", sockets, cores, cpus); >> + exit(1); >> + } >> + } >> + >
On 7/16/21 11:14 AM, Daniel P. Berrangé wrote: > On Fri, Jul 16, 2021 at 10:54:08AM +0200, Cornelia Huck wrote: >> On Wed, Jul 14 2021, Pierre Morel <pmorel@linux.ibm.com> wrote: >> >>> We need a s390x dedicated SMP parsing to handle s390x specificities. >>> >>> In this patch we only handle threads, cores and sockets for >>> s390x: >>> - do not support threads, we always have 1 single thread per core >>> - the sockets are filled one after the other with the cores >>> >>> Both these handlings are different from the standard smp_parse >>> functionement and reflect the CPU topology in the simple case >>> where all CPU belong to the same book. >>> >>> Topology levels above sockets, i.e. books, drawers, are not >>> considered at this stage and will be introduced in a later patch. >>> >>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>> --- >>> hw/s390x/s390-virtio-ccw.c | 42 ++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 42 insertions(+) >>> >>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>> index e4b18aef49..899d3a4137 100644 >>> --- a/hw/s390x/s390-virtio-ccw.c >>> +++ b/hw/s390x/s390-virtio-ccw.c >>> @@ -582,6 +582,47 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz) >>> return newsz; >>> } >>> >>> +/* >>> + * In S390CCW machine we do not support threads for now, >>> + * only sockets and cores. >>> + */ >>> +static void s390_smp_parse(MachineState *ms, QemuOpts *opts) >> >> It seems you based this on an older version of the code? The current >> signature of this function since 1e63fe685804 ("machine: pass QAPI >> struct to mc->smp_parse") is >> >> void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp); >> >> That affects your parsing, and also lets you get rid of the ugly exit(1) >> statements. >> >>> +{ >>> + unsigned cpus = qemu_opt_get_number(opts, "cpus", 1); >>> + unsigned sockets = qemu_opt_get_number(opts, "sockets", 1); >>> + unsigned cores = qemu_opt_get_number(opts, "cores", 1); >>> + >>> + if (opts) { >>> + if (cpus == 0 || sockets == 0 || cores == 0) { >> >> This behaviour looks different from what we do for other targets: if you >> specify the value as 0, a value is calculated from the other values; >> here, you error out. It's probably not a good idea to differ. > > I increasingly worry that we're making a mistake by going down the > route of having custom smp_parse implementations per target, as this > is showing signs of inconsistent behaviour and error reportings. I > think the differences / restrictions have granularity at a different > level that is being tested in many cases too. > > Whether threads != 1 is valid will likely vary depending on what > CPU model is chosen, rather than what architecture is chosen. > The same is true for dies != 1. We're not really checking this > closely even in x86 - for example I can request nonsense such > as a 25 year old i486 CPU model with hyperthreading and multiple > dies > > qemu-system-x86_64 -cpu 486 -smp 16,cores=4,dies=2,threads=2 > > In this patch, there is no error reporting if the user specifies > dies != 1 or threads != 1 - it just silently ignores the request > which is not good. yes, I should change this > > Some machine types may have constraints on CPU sockets. > > This can of course all be handled by custom smp_parse impls, but > this is ultimately going to lead to alot of duplicated and > inconsistent logic I fear. > > I wonder if we would be better off having machine class callback > that can report topology constraints for the current configuration, > along lines of > > smp_constraints(MachineState *ms, > int *max_sockets, > int *max_dies, > int *max_cores, > int *max_threads) I find the idee good, but what about making it really machine agnostic by removing names and using a generic smp_constraints(MachineState *ms, int *nb_levels, int *levels[] ); Level can be replaced by another name like container. The machine could also provide the level/container names according to its internal documentation. Regards, Pierre > > And then have only a single smp_parse impl that takes into account > these constraints, to report errors / fill in missing fields / etc ? > > Regards, > Daniel >
(restored cc:s) On Fri, Jul 16 2021, Pierre Morel <pmorel@linux.ibm.com> wrote: > On 7/16/21 11:14 AM, Daniel P. Berrangé wrote: >> I increasingly worry that we're making a mistake by going down the >> route of having custom smp_parse implementations per target, as this >> is showing signs of inconsistent behaviour and error reportings. I >> think the differences / restrictions have granularity at a different >> level that is being tested in many cases too. >> >> Whether threads != 1 is valid will likely vary depending on what >> CPU model is chosen, rather than what architecture is chosen. >> The same is true for dies != 1. We're not really checking this >> closely even in x86 - for example I can request nonsense such >> as a 25 year old i486 CPU model with hyperthreading and multiple >> dies >> >> qemu-system-x86_64 -cpu 486 -smp 16,cores=4,dies=2,threads=2 Now that's what I'd call an upgrade :) >> >> In this patch, there is no error reporting if the user specifies >> dies != 1 or threads != 1 - it just silently ignores the request >> which is not good. > > yes, I should change this > >> >> Some machine types may have constraints on CPU sockets. >> >> This can of course all be handled by custom smp_parse impls, but >> this is ultimately going to lead to alot of duplicated and >> inconsistent logic I fear. >> >> I wonder if we would be better off having machine class callback >> that can report topology constraints for the current configuration, >> along lines ofsmp_constraints(MachineState *ms, >> >> smp_constraints(MachineState *ms, >> int *max_sockets, >> int *max_dies, >> int *max_cores, >> int *max_threads) > > I find the idee good, but what about making it really machine agnostic > by removing names and using a generic > > smp_constraints(MachineState *ms, > int *nb_levels, > int *levels[] > ); > > Level can be replaced by another name like container. > The machine could also provide the level/container names according to > its internal documentation. In theory, this could give us more flexibility; however, wouldn't that still mean that the core needs to have some knowledge of the individual levels? We also have the command line parsing to consider, and that one uses concrete names (which may or may not make sense, depending on what machine you are trying to configure), and we'd still have to map these to 'levels'. > > Regards, > Pierre > > > >> >> And then have only a single smp_parse impl that takes into account >> these constraints, to report errors / fill in missing fields / etc ? >> >> Regards, >> Daniel >> > > -- > Pierre Morel > IBM Lab Boeblingen
On Mon, Jul 19, 2021 at 05:43:29PM +0200, Cornelia Huck wrote: > (restored cc:s) > > On Fri, Jul 16 2021, Pierre Morel <pmorel@linux.ibm.com> wrote: > > > On 7/16/21 11:14 AM, Daniel P. Berrangé wrote: > >> I increasingly worry that we're making a mistake by going down the > >> route of having custom smp_parse implementations per target, as this > >> is showing signs of inconsistent behaviour and error reportings. I > >> think the differences / restrictions have granularity at a different > >> level that is being tested in many cases too. > >> > >> Whether threads != 1 is valid will likely vary depending on what > >> CPU model is chosen, rather than what architecture is chosen. > >> The same is true for dies != 1. We're not really checking this > >> closely even in x86 - for example I can request nonsense such > >> as a 25 year old i486 CPU model with hyperthreading and multiple > >> dies > >> > >> qemu-system-x86_64 -cpu 486 -smp 16,cores=4,dies=2,threads=2 > > Now that's what I'd call an upgrade :) > > >> > >> In this patch, there is no error reporting if the user specifies > >> dies != 1 or threads != 1 - it just silently ignores the request > >> which is not good. > > > > yes, I should change this > > > >> > >> Some machine types may have constraints on CPU sockets. > >> > >> This can of course all be handled by custom smp_parse impls, but > >> this is ultimately going to lead to alot of duplicated and > >> inconsistent logic I fear. > >> > >> I wonder if we would be better off having machine class callback > >> that can report topology constraints for the current configuration, > >> along lines ofsmp_constraints(MachineState *ms, > >> > >> smp_constraints(MachineState *ms, > >> int *max_sockets, > >> int *max_dies, > >> int *max_cores, > >> int *max_threads) > > > > I find the idee good, but what about making it really machine agnostic > > by removing names and using a generic > > > > smp_constraints(MachineState *ms, > > int *nb_levels, > > int *levels[] > > ); > > > > Level can be replaced by another name like container. > > The machine could also provide the level/container names according to > > its internal documentation. > > In theory, this could give us more flexibility; however, wouldn't > that still mean that the core needs to have some knowledge of the > individual levels? We also have the command line parsing to consider, > and that one uses concrete names (which may or may not make sense, > depending on what machine you are trying to configure), and we'd still > have to map these to 'levels'. Yeah, we need to deal with names in several places, so I don't think abstracting it in one place is desirable, as it introduces the need to convert between the two and potentially obscures the semantics. Regards, Daniel
On 7/19/21 5:52 PM, Daniel P. Berrangé wrote: > On Mon, Jul 19, 2021 at 05:43:29PM +0200, Cornelia Huck wrote: >> (restored cc:s) >> >> On Fri, Jul 16 2021, Pierre Morel <pmorel@linux.ibm.com> wrote: >> >>> On 7/16/21 11:14 AM, Daniel P. Berrangé wrote: >>>> I increasingly worry that we're making a mistake by going down the >>>> route of having custom smp_parse implementations per target, as this >>>> is showing signs of inconsistent behaviour and error reportings. I >>>> think the differences / restrictions have granularity at a different >>>> level that is being tested in many cases too. >>>> >>>> Whether threads != 1 is valid will likely vary depending on what >>>> CPU model is chosen, rather than what architecture is chosen. >>>> The same is true for dies != 1. We're not really checking this >>>> closely even in x86 - for example I can request nonsense such >>>> as a 25 year old i486 CPU model with hyperthreading and multiple >>>> dies >>>> >>>> qemu-system-x86_64 -cpu 486 -smp 16,cores=4,dies=2,threads=2 >> >> Now that's what I'd call an upgrade :) >> >>>> >>>> In this patch, there is no error reporting if the user specifies >>>> dies != 1 or threads != 1 - it just silently ignores the request >>>> which is not good. >>> >>> yes, I should change this >>> >>>> >>>> Some machine types may have constraints on CPU sockets. >>>> >>>> This can of course all be handled by custom smp_parse impls, but >>>> this is ultimately going to lead to alot of duplicated and >>>> inconsistent logic I fear. >>>> >>>> I wonder if we would be better off having machine class callback >>>> that can report topology constraints for the current configuration, >>>> along lines ofsmp_constraints(MachineState *ms, >>>> >>>> smp_constraints(MachineState *ms, >>>> int *max_sockets, >>>> int *max_dies, >>>> int *max_cores, >>>> int *max_threads) >>> >>> I find the idee good, but what about making it really machine agnostic >>> by removing names and using a generic >>> >>> smp_constraints(MachineState *ms, >>> int *nb_levels, >>> int *levels[] >>> ); >>> >>> Level can be replaced by another name like container. >>> The machine could also provide the level/container names according to >>> its internal documentation. >> >> In theory, this could give us more flexibility; however, wouldn't >> that still mean that the core needs to have some knowledge of the >> individual levels? We also have the command line parsing to consider, >> and that one uses concrete names (which may or may not make sense, >> depending on what machine you are trying to configure), and we'd still >> have to map these to 'levels'. > > Yeah, we need to deal with names in several places, so I don't think > abstracting it in one place is desirable, as it introduces the need > to convert between the two and potentially obscures the semantics. > Converting with names looks possible to me, every architecture can export a topology_name array or structure indicating names and other informations like the maximum possible count of entries at this level. We have now the SMPConfiguration, couldn't we use it for this? Regards, Pierre > > Regards, > Daniel >
On 7/20/21 9:37 AM, Pierre Morel wrote: > > > On 7/19/21 5:52 PM, Daniel P. Berrangé wrote: >> On Mon, Jul 19, 2021 at 05:43:29PM +0200, Cornelia Huck wrote: >>> (restored cc:s) >>> >>> On Fri, Jul 16 2021, Pierre Morel <pmorel@linux.ibm.com> wrote: >>> >>>> On 7/16/21 11:14 AM, Daniel P. Berrangé wrote: >>>>> I increasingly worry that we're making a mistake by going down the >>>>> route of having custom smp_parse implementations per target, as this >>>>> is showing signs of inconsistent behaviour and error reportings. I >>>>> think the differences / restrictions have granularity at a different >>>>> level that is being tested in many cases too. >>>>> >>>>> Whether threads != 1 is valid will likely vary depending on what >>>>> CPU model is chosen, rather than what architecture is chosen. >>>>> The same is true for dies != 1. We're not really checking this >>>>> closely even in x86 - for example I can request nonsense such >>>>> as a 25 year old i486 CPU model with hyperthreading and multiple >>>>> dies >>>>> >>>>> qemu-system-x86_64 -cpu 486 -smp 16,cores=4,dies=2,threads=2 >>> >>> Now that's what I'd call an upgrade :) >>> >>>>> >>>>> In this patch, there is no error reporting if the user specifies >>>>> dies != 1 or threads != 1 - it just silently ignores the request >>>>> which is not good. >>>> >>>> yes, I should change this >>>> >>>>> >>>>> Some machine types may have constraints on CPU sockets. >>>>> >>>>> This can of course all be handled by custom smp_parse impls, but >>>>> this is ultimately going to lead to alot of duplicated and >>>>> inconsistent logic I fear. >>>>> >>>>> I wonder if we would be better off having machine class callback >>>>> that can report topology constraints for the current configuration, >>>>> along lines ofsmp_constraints(MachineState *ms, >>>>> >>>>> smp_constraints(MachineState *ms, >>>>> int *max_sockets, >>>>> int *max_dies, >>>>> int *max_cores, >>>>> int *max_threads) >>>> >>>> I find the idee good, but what about making it really machine agnostic >>>> by removing names and using a generic >>>> >>>> smp_constraints(MachineState *ms, >>>> int *nb_levels, >>>> int *levels[] >>>> ); >>>> >>>> Level can be replaced by another name like container. >>>> The machine could also provide the level/container names according to >>>> its internal documentation. >>> >>> In theory, this could give us more flexibility; however, wouldn't >>> that still mean that the core needs to have some knowledge of the >>> individual levels? We also have the command line parsing to consider, >>> and that one uses concrete names (which may or may not make sense, >>> depending on what machine you are trying to configure), and we'd still >>> have to map these to 'levels'. >> >> Yeah, we need to deal with names in several places, so I don't think >> abstracting it in one place is desirable, as it introduces the need >> to convert between the two and potentially obscures the semantics. >> > > Converting with names looks possible to me, every architecture can > export a topology_name array or structure indicating names and other > informations like the maximum possible count of entries at this level. > > We have now the SMPConfiguration, couldn't we use it for this? Hum, I think I over estimated my understanding of what json is capable of. Sorry, forget it.
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index e4b18aef49..899d3a4137 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -582,6 +582,47 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz) return newsz; } +/* + * In S390CCW machine we do not support threads for now, + * only sockets and cores. + */ +static void s390_smp_parse(MachineState *ms, QemuOpts *opts) +{ + unsigned cpus = qemu_opt_get_number(opts, "cpus", 1); + unsigned sockets = qemu_opt_get_number(opts, "sockets", 1); + unsigned cores = qemu_opt_get_number(opts, "cores", 1); + + if (opts) { + if (cpus == 0 || sockets == 0 || cores == 0) { + error_report("cpu topology: " + "sockets (%u), cores (%u) or number of CPU(%u) " + "can not be zero", sockets, cores, cpus); + exit(1); + } + } + + ms->smp.max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus); + if (ms->smp.max_cpus < cpus) { + error_report("maxcpus must be equal to or greater than smp"); + exit(1); + } + + if (!qemu_opt_find(opts, "cores")) { + cores = ms->smp.max_cpus / sockets; + } + + if (sockets * cores != ms->smp.max_cpus) { + error_report("Invalid CPU topology: sockets (%u) * cores (%u) " + "!= maxcpus (%u)", sockets, cores, ms->smp.max_cpus); + exit(1); + } + + ms->smp.threads = 1; + ms->smp.cpus = cpus; + ms->smp.cores = cores; + ms->smp.sockets = sockets; +} + static void ccw_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); @@ -612,6 +653,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) hc->unplug_request = s390_machine_device_unplug_request; nc->nmi_monitor_handler = s390_nmi; mc->default_ram_id = "s390.ram"; + mc->smp_parse = s390_smp_parse; } static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)
We need a s390x dedicated SMP parsing to handle s390x specificities. In this patch we only handle threads, cores and sockets for s390x: - do not support threads, we always have 1 single thread per core - the sockets are filled one after the other with the cores Both these handlings are different from the standard smp_parse functionement and reflect the CPU topology in the simple case where all CPU belong to the same book. Topology levels above sockets, i.e. books, drawers, are not considered at this stage and will be introduced in a later patch. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- hw/s390x/s390-virtio-ccw.c | 42 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)