[PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number
@ 2023-05-24 14:37 Clément Chigot
  2023-05-24 15:01 ` Francisco Iglesias
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Clément Chigot @ 2023-05-24 14:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-arm, edgar.iglesias, alistair, peter.maydell,
	Clément Chigot

When passing --smp with a number lower than XLNX_ZYNQMP_NUM_APU_CPUS,
the expression (ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS) will result
in a positive number as ms->smp.cpus is a unsigned int.
This will raise the following error afterwards, as Qemu will try to
instantiate some additional RPUs.
  | $ qemu-system-aarch64 --smp 1 -M xlnx-zcu102
  | **
  | ERROR:../src/tcg/tcg.c:777:tcg_register_thread:
  |   assertion failed: (n < tcg_max_ctxs)

Signed-off-by: Clément Chigot <chigot@adacore.com>
---
 hw/arm/xlnx-zynqmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 335cfc417d..5905a33015 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -213,7 +213,7 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
                                    const char *boot_cpu, Error **errp)
 {
     int i;
-    int num_rpus = MIN(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
+    int num_rpus = MIN((int)(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS),
                        XLNX_ZYNQMP_NUM_RPU_CPUS);
 
     if (num_rpus <= 0) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number
  2023-05-24 14:37 [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number Clément Chigot
@ 2023-05-24 15:01 ` Francisco Iglesias
  2023-05-24 15:06 ` Philippe Mathieu-Daudé
  2023-05-25 10:17 ` Peter Maydell
  2 siblings, 0 replies; 5+ messages in thread
From: Francisco Iglesias @ 2023-05-24 15:01 UTC (permalink / raw)
  To: Clément Chigot
  Cc: qemu-devel, qemu-arm, edgar.iglesias, alistair, peter.maydell

On [2023 May 24] Wed 16:37:14, Clément Chigot wrote:
> When passing --smp with a number lower than XLNX_ZYNQMP_NUM_APU_CPUS,
> the expression (ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS) will result
> in a positive number as ms->smp.cpus is a unsigned int.
> This will raise the following error afterwards, as Qemu will try to
> instantiate some additional RPUs.
>   | $ qemu-system-aarch64 --smp 1 -M xlnx-zcu102
>   | **
>   | ERROR:../src/tcg/tcg.c:777:tcg_register_thread:
>   |   assertion failed: (n < tcg_max_ctxs)
> 
> Signed-off-by: Clément Chigot <chigot@adacore.com>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>
Tested-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/arm/xlnx-zynqmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 335cfc417d..5905a33015 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -213,7 +213,7 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
>                                     const char *boot_cpu, Error **errp)
>  {
>      int i;
> -    int num_rpus = MIN(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
> +    int num_rpus = MIN((int)(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS),
>                         XLNX_ZYNQMP_NUM_RPU_CPUS);
>  
>      if (num_rpus <= 0) {
> -- 
> 2.25.1
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number
  2023-05-24 14:37 [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number Clément Chigot
  2023-05-24 15:01 ` Francisco Iglesias
@ 2023-05-24 15:06 ` Philippe Mathieu-Daudé
  2023-05-24 15:17   ` Clément Chigot
  2023-05-25 10:17 ` Peter Maydell
  2 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-05-24 15:06 UTC (permalink / raw)
  To: Clément Chigot, qemu-devel
  Cc: qemu-arm, edgar.iglesias, alistair, peter.maydell

Hi Clément,

On 24/5/23 16:37, Clément Chigot wrote:
> When passing --smp with a number lower than XLNX_ZYNQMP_NUM_APU_CPUS,
> the expression (ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS) will result
> in a positive number as ms->smp.cpus is a unsigned int.
> This will raise the following error afterwards, as Qemu will try to
> instantiate some additional RPUs.
>    | $ qemu-system-aarch64 --smp 1 -M xlnx-zcu102
>    | **
>    | ERROR:../src/tcg/tcg.c:777:tcg_register_thread:
>    |   assertion failed: (n < tcg_max_ctxs)
> 
> Signed-off-by: Clément Chigot <chigot@adacore.com>
> ---
>   hw/arm/xlnx-zynqmp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 335cfc417d..5905a33015 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -213,7 +213,7 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
>                                      const char *boot_cpu, Error **errp)
>   {
>       int i;
> -    int num_rpus = MIN(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
> +    int num_rpus = MIN((int)(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS),
>                          XLNX_ZYNQMP_NUM_RPU_CPUS);
>   
>       if (num_rpus <= 0) {

Can we set mc->min_cpus in xlnx_zcu102_machine_class_init() instead?

-- >8 --
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 4c84bb932a..60a2710e21 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -269,6 +269,7 @@ static void 
xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data)
      mc->block_default_type = IF_IDE;
      mc->units_per_default_bus = 1;
      mc->ignore_memory_transaction_failures = true;
+    mc->min_cpus = XLNX_ZYNQMP_NUM_APU_CPUS;
      mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + XLNX_ZYNQMP_NUM_RPU_CPUS;
      mc->default_cpus = XLNX_ZYNQMP_NUM_APU_CPUS;
      mc->default_ram_id = "ddr-ram";
---

$ qemu-system-aarch64 -M xlnx-zcu102 -smp 1
qemu-system-aarch64: Invalid SMP CPUs 1. The min CPUs supported by 
machine 'xlnx-zcu102' is 4

Regards,

Phil.

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number
  2023-05-24 15:06 ` Philippe Mathieu-Daudé
@ 2023-05-24 15:17   ` Clément Chigot
  0 siblings, 0 replies; 5+ messages in thread
From: Clément Chigot @ 2023-05-24 15:17 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, qemu-arm, edgar.iglesias, alistair, peter.maydell

On Wed, May 24, 2023 at 5:06 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi Clément,
>
> On 24/5/23 16:37, Clément Chigot wrote:
> > When passing --smp with a number lower than XLNX_ZYNQMP_NUM_APU_CPUS,
> > the expression (ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS) will result
> > in a positive number as ms->smp.cpus is a unsigned int.
> > This will raise the following error afterwards, as Qemu will try to
> > instantiate some additional RPUs.
> >    | $ qemu-system-aarch64 --smp 1 -M xlnx-zcu102
> >    | **
> >    | ERROR:../src/tcg/tcg.c:777:tcg_register_thread:
> >    |   assertion failed: (n < tcg_max_ctxs)
> >
> > Signed-off-by: Clément Chigot <chigot@adacore.com>
> > ---
> >   hw/arm/xlnx-zynqmp.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> > index 335cfc417d..5905a33015 100644
> > --- a/hw/arm/xlnx-zynqmp.c
> > +++ b/hw/arm/xlnx-zynqmp.c
> > @@ -213,7 +213,7 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
> >                                      const char *boot_cpu, Error **errp)
> >   {
> >       int i;
> > -    int num_rpus = MIN(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
> > +    int num_rpus = MIN((int)(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS),
> >                          XLNX_ZYNQMP_NUM_RPU_CPUS);
> >
> >       if (num_rpus <= 0) {
>
> Can we set mc->min_cpus in xlnx_zcu102_machine_class_init() instead?
>
> -- >8 --
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index 4c84bb932a..60a2710e21 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -269,6 +269,7 @@ static void
> xlnx_zcu102_machine_class_init(ObjectClass *oc, void *data)
>       mc->block_default_type = IF_IDE;
>       mc->units_per_default_bus = 1;
>       mc->ignore_memory_transaction_failures = true;
> +    mc->min_cpus = XLNX_ZYNQMP_NUM_APU_CPUS;
>       mc->max_cpus = XLNX_ZYNQMP_NUM_APU_CPUS + XLNX_ZYNQMP_NUM_RPU_CPUS;
>       mc->default_cpus = XLNX_ZYNQMP_NUM_APU_CPUS;
>       mc->default_ram_id = "ddr-ram";
> ---
>
> $ qemu-system-aarch64 -M xlnx-zcu102 -smp 1
> qemu-system-aarch64: Invalid SMP CPUs 1. The min CPUs supported by
> machine 'xlnx-zcu102' is 4

We encountered this issue when we were trying to emulate a zynqmp with
only two cortex-a53 cores. I don't have the full context but this was
probably to debug a race condition or something like that.
Thus, I would like to keep the possibility of lowering the CPU number,
even if it doesn't match the real board.

Clément

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number
  2023-05-24 14:37 [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number Clément Chigot
  2023-05-24 15:01 ` Francisco Iglesias
  2023-05-24 15:06 ` Philippe Mathieu-Daudé
@ 2023-05-25 10:17 ` Peter Maydell
  2 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2023-05-25 10:17 UTC (permalink / raw)
  To: Clément Chigot; +Cc: qemu-devel, qemu-arm, edgar.iglesias, alistair

On Wed, 24 May 2023 at 15:37, Clément Chigot <chigot@adacore.com> wrote:
>
> When passing --smp with a number lower than XLNX_ZYNQMP_NUM_APU_CPUS,
> the expression (ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS) will result
> in a positive number as ms->smp.cpus is a unsigned int.
> This will raise the following error afterwards, as Qemu will try to
> instantiate some additional RPUs.
>   | $ qemu-system-aarch64 --smp 1 -M xlnx-zcu102
>   | **
>   | ERROR:../src/tcg/tcg.c:777:tcg_register_thread:
>   |   assertion failed: (n < tcg_max_ctxs)
>
> Signed-off-by: Clément Chigot <chigot@adacore.com>
> ---
>  hw/arm/xlnx-zynqmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 335cfc417d..5905a33015 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -213,7 +213,7 @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s,
>                                     const char *boot_cpu, Error **errp)
>  {
>      int i;
> -    int num_rpus = MIN(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
> +    int num_rpus = MIN((int)(ms->smp.cpus - XLNX_ZYNQMP_NUM_APU_CPUS),
>                         XLNX_ZYNQMP_NUM_RPU_CPUS);

Applied to target-arm.next, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-05-25 10:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-24 14:37 [PATCH] hw/arm/xlnx-zynqmp: fix unsigned error when checking the RPUs number Clément Chigot
2023-05-24 15:01 ` Francisco Iglesias
2023-05-24 15:06 ` Philippe Mathieu-Daudé
2023-05-24 15:17   ` Clément Chigot
2023-05-25 10:17 ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.