| Message ID | 160343848141.8350.10469322440262034340.stgit@pasha-ThinkPad-X280 |
|---|---|
| State | New |
| Headers | show |
| Series | cpus: verify that number of created cpus do not exceed smp params | expand |
On 10/23/20 9:34 AM, Pavel Dovgalyuk wrote: > Machine definitions may miss some vCPU-related parameters. > E.g., xlnx-versal-virt missed min_cpus and it was set to 1 by default. > This allowed using -smp 1 command line argument. But the machine > still created 2 vCPUs and passed all checks. > This patch adds one more check that does not allow creating > extra cpus that exceed the values specified in machine/smp. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> > --- > 0 files changed > > diff --git a/softmmu/cpus.c b/softmmu/cpus.c > index 47cceddd80..da74794e09 100644 > --- a/softmmu/cpus.c > +++ b/softmmu/cpus.c > @@ -603,6 +603,11 @@ void qemu_init_vcpu(CPUState *cpu) > { > MachineState *ms = MACHINE(qdev_get_machine()); > > + if (cpu->cpu_index >= ms->smp.cpus) { > + fprintf(stderr, "Machine definition error: trying to create too many CPUs\n"); > + exit(1); Shouldn't this be an assert()? > + } > + > cpu->nr_cores = ms->smp.cores; > cpu->nr_threads = ms->smp.threads; > cpu->stopped = true; > >
On 23.10.2020 11:10, Philippe Mathieu-Daudé wrote: > On 10/23/20 9:34 AM, Pavel Dovgalyuk wrote: >> Machine definitions may miss some vCPU-related parameters. >> E.g., xlnx-versal-virt missed min_cpus and it was set to 1 by default. >> This allowed using -smp 1 command line argument. But the machine >> still created 2 vCPUs and passed all checks. >> This patch adds one more check that does not allow creating >> extra cpus that exceed the values specified in machine/smp. >> >> Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> >> --- >> 0 files changed >> >> diff --git a/softmmu/cpus.c b/softmmu/cpus.c >> index 47cceddd80..da74794e09 100644 >> --- a/softmmu/cpus.c >> +++ b/softmmu/cpus.c >> @@ -603,6 +603,11 @@ void qemu_init_vcpu(CPUState *cpu) >> { >> MachineState *ms = MACHINE(qdev_get_machine()); >> + if (cpu->cpu_index >= ms->smp.cpus) { >> + fprintf(stderr, "Machine definition error: trying to create >> too many CPUs\n"); >> + exit(1); > > Shouldn't this be an assert()? I thought about this. Bugs could be unnoticed and reveal in release. Therefore user should know the details and shouldn't see an assert. > >> + } >> + >> cpu->nr_cores = ms->smp.cores; >> cpu->nr_threads = ms->smp.threads; >> cpu->stopped = true; >> >> >
On Fri, 23 Oct 2020 10:34:41 +0300 Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> wrote: > Machine definitions may miss some vCPU-related parameters. > E.g., xlnx-versal-virt missed min_cpus and it was set to 1 by default. > This allowed using -smp 1 command line argument. But the machine > still created 2 vCPUs and passed all checks. > This patch adds one more check that does not allow creating > extra cpus that exceed the values specified in machine/smp. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> > --- > 0 files changed > > diff --git a/softmmu/cpus.c b/softmmu/cpus.c > index 47cceddd80..da74794e09 100644 > --- a/softmmu/cpus.c > +++ b/softmmu/cpus.c > @@ -603,6 +603,11 @@ void qemu_init_vcpu(CPUState *cpu) > { > MachineState *ms = MACHINE(qdev_get_machine()); > > + if (cpu->cpu_index >= ms->smp.cpus) { looks like for such machines we need MachineClass:min_cpus + setting it in affected machines and corresponding check in smp_parse(), instead of terminating from qemu_init_vcpu(); > + fprintf(stderr, "Machine definition error: trying to create too many CPUs\n"); > + exit(1); > + } > + > cpu->nr_cores = ms->smp.cores; > cpu->nr_threads = ms->smp.threads; > cpu->stopped = true; > >
diff --git a/softmmu/cpus.c b/softmmu/cpus.c index 47cceddd80..da74794e09 100644 --- a/softmmu/cpus.c +++ b/softmmu/cpus.c @@ -603,6 +603,11 @@ void qemu_init_vcpu(CPUState *cpu) { MachineState *ms = MACHINE(qdev_get_machine()); + if (cpu->cpu_index >= ms->smp.cpus) { + fprintf(stderr, "Machine definition error: trying to create too many CPUs\n"); + exit(1); + } + cpu->nr_cores = ms->smp.cores; cpu->nr_threads = ms->smp.threads; cpu->stopped = true;
Machine definitions may miss some vCPU-related parameters. E.g., xlnx-versal-virt missed min_cpus and it was set to 1 by default. This allowed using -smp 1 command line argument. But the machine still created 2 vCPUs and passed all checks. This patch adds one more check that does not allow creating extra cpus that exceed the values specified in machine/smp. Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> --- 0 files changed