[Qemu-devel] [PATCH] hw/core/generic-loader: Allow PC to be set on command line

[Qemu-devel] [PATCH] hw/core/generic-loader: Allow PC to be set on command line

[Peter Maydell]

The documentation for the generic loader claims that you can
set the PC for a CPU with an option of the form
  -device loader,cpu-num=0,addr=0x10000004

However if you try this QEMU complains:
  cpu_num must be specified when setting a program counter

This is because we were testing against 0 rather than CPU_NONE.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I've also noticed that you can't use this to specify that
the starting address should be in Thumb mode for Arm CPUs,
but I'm not so sure of the right way to fix that...

 hw/core/generic-loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
index 46012673c3..cb0e68486d 100644
--- a/hw/core/generic-loader.c
+++ b/hw/core/generic-loader.c
@@ -105,7 +105,7 @@ static void generic_loader_realize(DeviceState *dev, Error **errp)
             error_setg(errp, "data can not be specified when setting a "
                        "program counter");
             return;
-        } else if (!s->cpu_num) {
+        } else if (s->cpu_num == CPU_NONE) {
             error_setg(errp, "cpu_num must be specified when setting a "
                        "program counter");
             return;
-- 
2.16.1

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Allow PC to be set on command line

[Alistair Francis]

On Mon, Feb 5, 2018 at 7:04 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The documentation for the generic loader claims that you can
> set the PC for a CPU with an option of the form
>   -device loader,cpu-num=0,addr=0x10000004
>
> However if you try this QEMU complains:
>   cpu_num must be specified when setting a program counter
>
> This is because we were testing against 0 rather than CPU_NONE.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

> ---
> I've also noticed that you can't use this to specify that
> the starting address should be in Thumb mode for Arm CPUs,
> but I'm not so sure of the right way to fix that...

Should we at least update the documentation to mention this then?

Alistair

>
>  hw/core/generic-loader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
> index 46012673c3..cb0e68486d 100644
> --- a/hw/core/generic-loader.c
> +++ b/hw/core/generic-loader.c
> @@ -105,7 +105,7 @@ static void generic_loader_realize(DeviceState *dev, Error **errp)
>              error_setg(errp, "data can not be specified when setting a "
>                         "program counter");
>              return;
> -        } else if (!s->cpu_num) {
> +        } else if (s->cpu_num == CPU_NONE) {
>              error_setg(errp, "cpu_num must be specified when setting a "
>                         "program counter");
>              return;
> --
> 2.16.1
>
>

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Allow PC to be set on command line

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 1503 bytes --]

On 02/05/2018 12:04 PM, Peter Maydell wrote:
> The documentation for the generic loader claims that you can
> set the PC for a CPU with an option of the form
>   -device loader,cpu-num=0,addr=0x10000004
> 
> However if you try this QEMU complains:
>   cpu_num must be specified when setting a program counter
> 
> This is because we were testing against 0 rather than CPU_NONE.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I've also noticed that you can't use this to specify that
> the starting address should be in Thumb mode for Arm CPUs,
> but I'm not so sure of the right way to fix that...

Can using an impair addr work? (no arch-specific parser)

> 
>  hw/core/generic-loader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
> index 46012673c3..cb0e68486d 100644
> --- a/hw/core/generic-loader.c
> +++ b/hw/core/generic-loader.c
> @@ -105,7 +105,7 @@ static void generic_loader_realize(DeviceState *dev, Error **errp)
>              error_setg(errp, "data can not be specified when setting a "
>                         "program counter");
>              return;
> -        } else if (!s->cpu_num) {
> +        } else if (s->cpu_num == CPU_NONE) {
>              error_setg(errp, "cpu_num must be specified when setting a "
>                         "program counter");
>              return;
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]