| Message ID | 1436224445-19449-1-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
Am 07.07.2015 um 01:13 schrieb Andreas Färber: > Hello Peter, > > This is my QOM CPU patch queue. Please pull. > > Note: For time reasons I did not give this queue as much testing as usual, > in particular BSD and non-x86 KVM hosts were not covered. > > Regards, > Andreas > > Cc: Peter Maydell <peter.maydell@linaro.org> > > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > The following changes since commit 7edd8e4660beb301d527257f8e04ebec0f841cb0: > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-07-06 14:03:44 +0100) > > are available in the git repository at: > > git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-peter > > for you to fetch changes up to 116382f1504d655a1afdf3eac18d276a200428b7: > > disas: cris: QOMify target specific disas setup (2015-07-06 22:36:17 +0200) > > ---------------------------------------------------------------- > QOM CPUState and X86CPU > > * Further QOM'ification of CPU initialization > * Propagation of CPUState arguments and elimination of ENV_GET_CPU() usage > * cpu_set_pc() abstraction > * CPUClass::disas_set_info() hook > > ---------------------------------------------------------------- [...] Self-nack, hurry is never good: /aarch64/qom/xlnx-ep108: qemu-system-aarch64: Trying to use more CPUs than allowed max of 1 Broken pipe FAIL Peter C., any ideas why this is regressing? Sorry, Andreas
On Mon, Jul 6, 2015 at 5:24 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 07.07.2015 um 01:13 schrieb Andreas Färber: >> Hello Peter, >> >> This is my QOM CPU patch queue. Please pull. >> >> Note: For time reasons I did not give this queue as much testing as usual, >> in particular BSD and non-x86 KVM hosts were not covered. >> >> Regards, >> Andreas >> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> >> Cc: Eduardo Habkost <ehabkost@redhat.com> >> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> >> The following changes since commit 7edd8e4660beb301d527257f8e04ebec0f841cb0: >> >> Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-07-06 14:03:44 +0100) >> >> are available in the git repository at: >> >> git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-peter >> >> for you to fetch changes up to 116382f1504d655a1afdf3eac18d276a200428b7: >> >> disas: cris: QOMify target specific disas setup (2015-07-06 22:36:17 +0200) >> >> ---------------------------------------------------------------- >> QOM CPUState and X86CPU >> >> * Further QOM'ification of CPU initialization >> * Propagation of CPUState arguments and elimination of ENV_GET_CPU() usage >> * cpu_set_pc() abstraction >> * CPUClass::disas_set_info() hook >> >> ---------------------------------------------------------------- > [...] > > Self-nack, hurry is never good: > > /aarch64/qom/xlnx-ep108: > qemu-system-aarch64: Trying to use more CPUs than allowed max of 1 > Broken pipe > FAIL > > Peter C., any ideas why this is regressing? > This: + if (cpu >= max_cpus) { + error_setg(errp, "Trying to use more CPUs than allowed max of %d\n", + max_cpus); + return -1; xlnx-ep108 doesn't care about the -smp argument, it creates all 6 CPUs regardless of -smp. This is because the number of CPUs is not flexible in reality. It is also a heterogeneous arch (with R5s abd A53s) so trying to limit the grand total of CPUs is ambiguous (do you remove a53s or r5s for -smp < 6?). Can this check be dropped or is this a bug in xlnx where we should overcome by just forcing smp_cpus = 6 at machine level? Regards, Peter > Sorry, > Andreas > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB > 21284 (AG Nürnberg) >
On Tue, Jul 7, 2015 at 7:55 AM, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Mon, Jul 6, 2015 at 5:24 PM, Andreas Färber <afaerber@suse.de> wrote: >> Am 07.07.2015 um 01:13 schrieb Andreas Färber: >>> Hello Peter, >>> >>> This is my QOM CPU patch queue. Please pull. >>> >>> Note: For time reasons I did not give this queue as much testing as usual, >>> in particular BSD and non-x86 KVM hosts were not covered. >>> >>> Regards, >>> Andreas >>> >>> Cc: Peter Maydell <peter.maydell@linaro.org> >>> >>> Cc: Eduardo Habkost <ehabkost@redhat.com> >>> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >>> >>> The following changes since commit 7edd8e4660beb301d527257f8e04ebec0f841cb0: >>> >>> Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-07-06 14:03:44 +0100) >>> >>> are available in the git repository at: >>> >>> git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-peter >>> >>> for you to fetch changes up to 116382f1504d655a1afdf3eac18d276a200428b7: >>> >>> disas: cris: QOMify target specific disas setup (2015-07-06 22:36:17 +0200) >>> >>> ---------------------------------------------------------------- >>> QOM CPUState and X86CPU >>> >>> * Further QOM'ification of CPU initialization >>> * Propagation of CPUState arguments and elimination of ENV_GET_CPU() usage >>> * cpu_set_pc() abstraction >>> * CPUClass::disas_set_info() hook >>> >>> ---------------------------------------------------------------- >> [...] >> >> Self-nack, hurry is never good: >> >> /aarch64/qom/xlnx-ep108: >> qemu-system-aarch64: Trying to use more CPUs than allowed max of 1 >> Broken pipe >> FAIL >> >> Peter C., any ideas why this is regressing? >> > > This: > > + if (cpu >= max_cpus) { > + error_setg(errp, "Trying to use more CPUs than allowed max of %d\n", > + max_cpus); > + return -1; > > xlnx-ep108 doesn't care about the -smp argument, it creates all 6 CPUs > regardless of -smp. This is because the number of CPUs is not flexible > in reality. It is also a heterogeneous arch (with R5s abd A53s) so > trying to limit the grand total of CPUs is ambiguous (do you remove > a53s or r5s for -smp < 6?). > > Can this check be dropped or is this a bug in xlnx where we should > overcome by just forcing smp_cpus = 6 at machine level? That check is needed to fail CPU realization when an attempt is made to relialize (eg. via hotplug) more than allowed max number of CPUs. Regards, Bharata.
On Mon, Jul 6, 2015 at 7:25 PM, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Mon, Jul 6, 2015 at 5:24 PM, Andreas Färber <afaerber@suse.de> wrote: >> Am 07.07.2015 um 01:13 schrieb Andreas Färber: >>> Hello Peter, >>> >>> This is my QOM CPU patch queue. Please pull. >>> >>> Note: For time reasons I did not give this queue as much testing as usual, >>> in particular BSD and non-x86 KVM hosts were not covered. >>> >>> Regards, >>> Andreas >>> >>> Cc: Peter Maydell <peter.maydell@linaro.org> >>> >>> Cc: Eduardo Habkost <ehabkost@redhat.com> >>> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >>> >>> The following changes since commit 7edd8e4660beb301d527257f8e04ebec0f841cb0: >>> >>> Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-07-06 14:03:44 +0100) >>> >>> are available in the git repository at: >>> >>> git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-peter >>> >>> for you to fetch changes up to 116382f1504d655a1afdf3eac18d276a200428b7: >>> >>> disas: cris: QOMify target specific disas setup (2015-07-06 22:36:17 +0200) >>> >>> ---------------------------------------------------------------- >>> QOM CPUState and X86CPU >>> >>> * Further QOM'ification of CPU initialization >>> * Propagation of CPUState arguments and elimination of ENV_GET_CPU() usage >>> * cpu_set_pc() abstraction >>> * CPUClass::disas_set_info() hook >>> >>> ---------------------------------------------------------------- >> [...] >> >> Self-nack, hurry is never good: >> >> /aarch64/qom/xlnx-ep108: >> qemu-system-aarch64: Trying to use more CPUs than allowed max of 1 >> Broken pipe >> FAIL >> >> Peter C., any ideas why this is regressing? >> > > This: > > + if (cpu >= max_cpus) { > + error_setg(errp, "Trying to use more CPUs than allowed max of %d\n", > + max_cpus); > + return -1; > > xlnx-ep108 doesn't care about the -smp argument, it creates all 6 CPUs > regardless of -smp. This is because the number of CPUs is not flexible > in reality. It is also a heterogeneous arch (with R5s abd A53s) so > trying to limit the grand total of CPUs is ambiguous (do you remove > a53s or r5s for -smp < 6?). > > Can this check be dropped or is this a bug in xlnx where we should > overcome by just forcing smp_cpus = 6 at machine level? > > Regards, > Peter > This is the fix (patch 5): @@ -531,11 +531,11 @@ static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS); static int cpu_get_free_index(Error **errp) { - int cpu = find_first_zero_bit(cpu_index_map, max_cpus); + int cpu = find_first_zero_bit(cpu_index_map, MAX_CPUMASK_BITS); - if (cpu >= max_cpus) { - error_setg(errp, "Trying to use more CPUs than allowed max of %d", - max_cpus); + if (cpu >= MAX_CPUMASK_BITS) { + error_setg(errp, "Trying to use more CPUs than max of %d", + MAX_CPUMASK_BITS); return -1; } My thinking is, that the existing linear allocator for cpu-indicies does not have max_cpus based maximum index enforcement and adding it is not necessarily in the scope of Bharata's patch series. Given than this extra feature regresses without being critical to PPC CPU hotplug removal, we can just remove it from the patch and instead only enforce that the indicies are within the hard limits of the bitmap itself. We can follow-up with a more correct max check once we have the needed fix to either ep108 or core code, but this edit is hopefully enough for this PULL? An amended version of the PULL with the fix squashed in is available at: https://github.com/pcrost/qemu.git qom-cpu-for-peter Regards, Peter > >> Sorry, >> Andreas >> >> -- >> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB >> 21284 (AG Nürnberg) >>
On Mon, Jul 6, 2015 at 8:56 PM, Bharata B Rao <bharata.rao@gmail.com> wrote: > On Tue, Jul 7, 2015 at 7:55 AM, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> On Mon, Jul 6, 2015 at 5:24 PM, Andreas Färber <afaerber@suse.de> wrote: >>> Am 07.07.2015 um 01:13 schrieb Andreas Färber: >>>> Hello Peter, >>>> >>>> This is my QOM CPU patch queue. Please pull. >>>> >>>> Note: For time reasons I did not give this queue as much testing as usual, >>>> in particular BSD and non-x86 KVM hosts were not covered. >>>> >>>> Regards, >>>> Andreas >>>> >>>> Cc: Peter Maydell <peter.maydell@linaro.org> >>>> >>>> Cc: Eduardo Habkost <ehabkost@redhat.com> >>>> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >>>> >>>> The following changes since commit 7edd8e4660beb301d527257f8e04ebec0f841cb0: >>>> >>>> Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-07-06 14:03:44 +0100) >>>> >>>> are available in the git repository at: >>>> >>>> git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-peter >>>> >>>> for you to fetch changes up to 116382f1504d655a1afdf3eac18d276a200428b7: >>>> >>>> disas: cris: QOMify target specific disas setup (2015-07-06 22:36:17 +0200) >>>> >>>> ---------------------------------------------------------------- >>>> QOM CPUState and X86CPU >>>> >>>> * Further QOM'ification of CPU initialization >>>> * Propagation of CPUState arguments and elimination of ENV_GET_CPU() usage >>>> * cpu_set_pc() abstraction >>>> * CPUClass::disas_set_info() hook >>>> >>>> ---------------------------------------------------------------- >>> [...] >>> >>> Self-nack, hurry is never good: >>> >>> /aarch64/qom/xlnx-ep108: >>> qemu-system-aarch64: Trying to use more CPUs than allowed max of 1 >>> Broken pipe >>> FAIL >>> >>> Peter C., any ideas why this is regressing? >>> >> >> This: >> >> + if (cpu >= max_cpus) { >> + error_setg(errp, "Trying to use more CPUs than allowed max of %d\n", >> + max_cpus); >> + return -1; >> >> xlnx-ep108 doesn't care about the -smp argument, it creates all 6 CPUs >> regardless of -smp. This is because the number of CPUs is not flexible >> in reality. It is also a heterogeneous arch (with R5s abd A53s) so >> trying to limit the grand total of CPUs is ambiguous (do you remove >> a53s or r5s for -smp < 6?). >> >> Can this check be dropped or is this a bug in xlnx where we should >> overcome by just forcing smp_cpus = 6 at machine level? > > That check is needed to fail CPU realization when an attempt is made > to relialize (eg. via hotplug) more than allowed max number of CPUs. > What was the behaviour before this patch series for this case? I can't see any check in original code. had another read of your commit messages. It seems the main purpose of your change is to ensure CPU indicies are unique rather than within any limits so this new check is secondary to what you were trying to do. Can we just drop it? Regards, Peter > Regards, > Bharata. >
Hello Peter, This is my QOM CPU patch queue. Please pull. Note: For time reasons I did not give this queue as much testing as usual, in particular BSD and non-x86 KVM hosts were not covered. Regards, Andreas Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com> The following changes since commit 7edd8e4660beb301d527257f8e04ebec0f841cb0: Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2015-07-06 14:03:44 +0100) are available in the git repository at: git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-peter for you to fetch changes up to 116382f1504d655a1afdf3eac18d276a200428b7: disas: cris: QOMify target specific disas setup (2015-07-06 22:36:17 +0200) ---------------------------------------------------------------- QOM CPUState and X86CPU * Further QOM'ification of CPU initialization * Propagation of CPUState arguments and elimination of ENV_GET_CPU() usage * cpu_set_pc() abstraction * CPUClass::disas_set_info() hook ---------------------------------------------------------------- Bharata B Rao (3): cpu: Add Error argument to cpu_exec_init() cpu: Convert cpu_index into a bitmap target-ppc: Move cpu_exec_init() call to realize function Eduardo Habkost (3): cpu: No need to zero-initialize CPUState::numa_node cpu: Initialize breakpoint/watchpoint lists in cpu_common_initfn() cpu: Reorder cpu->as, cpu->thread_id, cpu->memory_dispatch init Peter Crosthwaite (16): translate-all: Change tb_flush() env argument to cpu gdbstub: Change gdbserver_fork() to accept cpu instead of env cpu: Change tcg_cpu_exec() arg to cpu, not env cpu: Change cpu_exec_init() arg to cpu, not env cpu-exec: Purge all uses of ENV_GET_CPU() cpu: Add wrapper for the set_pc() hook gdbstub: Use cpu_set_pc() helper hw/arm/boot: Use cpu_set_pc() microblaze: boot: Use cpu_set_pc() disas: Add print_insn to disassemble info disas: QOMify target specific setup disas: arm-a64: Make printfer and stream variable disas: arm: QOMify target specific disas setup disas: microblaze: QOMify target specific disas setup disas: cris: Fix 0 buffer length case disas: cris: QOMify target specific disas setup bsd-user/main.c | 6 ++- cpu-exec.c | 28 +++++------ cpus.c | 8 ++- disas.c | 119 ++++++++++++++++---------------------------- disas/arm-a64.cc | 22 ++++++-- disas/cris.c | 6 +-- exec.c | 71 ++++++++++++++++++++------ gdbstub.c | 14 ++---- hw/arm/boot.c | 24 ++++----- hw/microblaze/boot.c | 5 +- include/disas/bfd.h | 6 +++ include/exec/exec-all.h | 4 +- include/exec/gdbstub.h | 2 +- include/qom/cpu.h | 19 +++++++ linux-user/main.c | 30 +++++------ linux-user/signal.c | 2 +- qom/cpu.c | 9 ++++ target-alpha/cpu.c | 2 +- target-alpha/cpu.h | 2 +- target-alpha/sys_helper.c | 2 +- target-arm/cpu.c | 37 +++++++++++++- target-arm/cpu.h | 2 +- target-cris/cpu.c | 18 ++++++- target-cris/cpu.h | 2 +- target-i386/cpu.c | 2 +- target-i386/cpu.h | 2 +- target-i386/translate.c | 2 +- target-lm32/cpu.c | 2 +- target-lm32/cpu.h | 2 +- target-m68k/cpu.c | 2 +- target-m68k/cpu.h | 2 +- target-microblaze/cpu.c | 10 +++- target-microblaze/cpu.h | 2 +- target-mips/cpu.c | 2 +- target-mips/cpu.h | 2 +- target-moxie/cpu.c | 2 +- target-moxie/cpu.h | 2 +- target-openrisc/cpu.c | 2 +- target-openrisc/cpu.h | 2 +- target-ppc/cpu.h | 2 +- target-ppc/translate_init.c | 9 +++- target-s390x/cpu.c | 2 +- target-s390x/cpu.h | 2 +- target-sh4/cpu.c | 2 +- target-sh4/cpu.h | 2 +- target-sparc/cpu.c | 2 +- target-sparc/cpu.h | 2 +- target-tricore/cpu.c | 2 +- target-tricore/cpu.h | 2 +- target-unicore32/cpu.c | 2 +- target-unicore32/cpu.h | 3 +- target-xtensa/cpu.c | 2 +- target-xtensa/cpu.h | 2 +- translate-all.c | 6 +-- 54 files changed, 315 insertions(+), 205 deletions(-)