* [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs
@ 2019-01-21 18:43 Peter Maydell
2019-01-21 19:32 ` Edgar E. Iglesias
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Peter Maydell @ 2019-01-21 18:43 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: patches, Luc Michel, Alistair Francis, Edgar E. Iglesias
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.maydell@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.maydell@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);
+ if (num_rpus == 0) {
+ /* Don't create rpu-cluster object if there's nothing to put in it */
+ return;
+ }
+
object_initialize_child(OBJECT(s), "rpu-cluster", &s->rpu_cluster,
sizeof(s->rpu_cluster), TYPE_CPU_CLUSTER,
&error_abort, NULL);
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs
2019-01-21 18:43 [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs Peter Maydell
@ 2019-01-21 19:32 ` Edgar E. Iglesias
2019-01-21 20:12 ` Philippe Mathieu-Daudé
2019-01-24 14:12 ` Peter Maydell
2 siblings, 0 replies; 6+ messages in thread
From: Edgar E. Iglesias @ 2019-01-21 19:32 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel, patches, Luc Michel, Alistair Francis
On Mon, Jan 21, 2019 at 06:43:14PM +0000, 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.
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
> Signed-off-by: Peter Maydell <peter.maydell@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.maydell@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);
>
> + if (num_rpus == 0) {
> + /* Don't create rpu-cluster object if there's nothing to put in it */
> + return;
> + }
> +
> object_initialize_child(OBJECT(s), "rpu-cluster", &s->rpu_cluster,
> sizeof(s->rpu_cluster), TYPE_CPU_CLUSTER,
> &error_abort, NULL);
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs
2019-01-21 18:43 [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs Peter Maydell
2019-01-21 19:32 ` Edgar E. Iglesias
@ 2019-01-21 20:12 ` Philippe Mathieu-Daudé
2019-01-22 9:28 ` Peter Maydell
2019-01-24 14:12 ` Peter Maydell
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-01-21 20:12 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, Edgar E. Iglesias, Alistair Francis
Cc: qemu-devel, Luc Michel, patches
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.maydell@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.maydell@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", ...
> + /* Don't create rpu-cluster object if there's nothing to put in it */
> + return;
> + }
> +
> object_initialize_child(OBJECT(s), "rpu-cluster", &s->rpu_cluster,
> sizeof(s->rpu_cluster), TYPE_CPU_CLUSTER,
> &error_abort, NULL);
>
... because the rpu cluster is still created:
aarch64-softmmu/qemu-system-aarch64 -M xlnx-zcu102 -smp 2 -S -monitor stdio
(qemu) info qom-tree
/machine (xlnx-zcu102-machine)
/peripheral (container)
/soc (xlnx,zynqmp)
/rpu-cluster (cpu-cluster)
/apu-cluster (cpu-cluster)
/apu-cpu[1] (cortex-a53-arm-cpu)
/unnamed-gpio-in[1] (irq)
/unnamed-gpio-in[3] (irq)
/unnamed-gpio-in[2] (irq)
/unnamed-gpio-in[0] (irq)
/apu-cpu[0] (cortex-a53-arm-cpu)
/unnamed-gpio-in[1] (irq)
/unnamed-gpio-in[3] (irq)
/unnamed-gpio-in[2] (irq)
/unnamed-gpio-in[0] (irq)
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;
+ }
}
if (!s->boot_cpu_ptr) {
---
Regards,
Phil.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs
2019-01-21 20:12 ` Philippe Mathieu-Daudé
@ 2019-01-22 9:28 ` Peter Maydell
2019-01-22 9:43 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2019-01-22 9:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-arm, Edgar E. Iglesias, Alistair Francis, QEMU Developers,
Luc Michel, patches@linaro.org
On Mon, 21 Jan 2019 at 20:12, Philippe Mathieu-Daudé <philmd@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.maydell@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.maydell@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs
2019-01-22 9:28 ` Peter Maydell
@ 2019-01-22 9:43 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-01-22 9:43 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-arm, Edgar E. Iglesias, Alistair Francis, QEMU Developers,
Luc Michel, patches@linaro.org
On 1/22/19 10:28 AM, Peter Maydell wrote:
> On Mon, 21 Jan 2019 at 20:12, Philippe Mathieu-Daudé <philmd@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.maydell@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.maydell@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.
OK, whichever patch you prefer, you can add:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Regards,
Phil.
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs
2019-01-21 18:43 [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs Peter Maydell
2019-01-21 19:32 ` Edgar E. Iglesias
2019-01-21 20:12 ` Philippe Mathieu-Daudé
@ 2019-01-24 14:12 ` Peter Maydell
2 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2019-01-24 14:12 UTC (permalink / raw)
To: qemu-arm, QEMU Developers
Cc: patches@linaro.org, Luc Michel, Alistair Francis,
Edgar E. Iglesias
On Mon, 21 Jan 2019 at 18:43, Peter Maydell <peter.maydell@linaro.org> 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.maydell@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.maydell@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);
>
> + if (num_rpus == 0) {
> + /* Don't create rpu-cluster object if there's nothing to put in it */
> + return;
> + }
Applied to target-arm.next with a fixup to test for "<= 0".
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-24 14:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-21 18:43 [Qemu-devel] [PATCH] xlnx-zynqmp: Don't create rpu-cluster if there are no RPUs Peter Maydell
2019-01-21 19:32 ` Edgar E. Iglesias
2019-01-21 20:12 ` Philippe Mathieu-Daudé
2019-01-22 9:28 ` Peter Maydell
2019-01-22 9:43 ` Philippe Mathieu-Daudé
2019-01-24 14:12 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).