On Mon, 21 Jan 2019 at 20:12, Philippe Mathieu-Daudé <phi...@redhat.com> wrote: > > Hi Peter, > > On 1/21/19 7:43 PM, Peter Maydell wrote: > > If we aren't going to create any RPUs, then don't create the > > rpu-cluster unit. This allows us to add an assertion to the > > cluster object that it contains at least one CPU, which helps > > to avoid bugs in creating clusters and putting CPUs in them. > > > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > > --- > > This is a preparatory patch that is necessary for the series > > "[PATCH v3 0/4] tcg: support heterogenous CPU clusters" > > (20190121152218.9592-1-peter.mayd...@linaro.org) > > in order to avoid the xlnx-zcu102 board asserting if started with > > fewer than 5 CPUs. > > > > hw/arm/xlnx-zynqmp.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c > > index 370b0e44a38..16cba433cb7 100644 > > --- a/hw/arm/xlnx-zynqmp.c > > +++ b/hw/arm/xlnx-zynqmp.c > > @@ -178,6 +178,11 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, > > const char *boot_cpu, > > int i; > > int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, > > XLNX_ZYNQMP_NUM_RPU_CPUS); > > Not related to this patch, but this check seems dangerous, i.e. using > "-smp 2" we get num_rpus=-2 which luckyly doesn't enter the for() loop. > > > > > + if (num_rpus == 0) { > > With the current codebase, you'd have to check for "num_rpus <= 0", ...
Oops, nice catch. > What about this instead? > > -- >8 -- > @@ -451,10 +451,12 @@ static void xlnx_zynqmp_realize(DeviceState *dev, > Error **errp) > "RPUs just use -smp 6."); > } > > - xlnx_zynqmp_create_rpu(s, boot_cpu, &err); > - if (err) { > - error_propagate(errp, err); > - return; > + if (smp_cpus > XLNX_ZYNQMP_NUM_APU_CPUS) { > + xlnx_zynqmp_create_rpu(s, boot_cpu, &err); > + if (err) { > + error_propagate(errp, err); > + return; > + } > } Yeah, that would work too. I think I would just go for using "if (num_rpus <= 0)" in the function, though. thanks -- PMM