All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks
@ 2020-02-27 22:01 Niek Linnenbank
  2020-02-27 22:01 ` [PATCH v1 1/4] hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine definition Niek Linnenbank
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Niek Linnenbank @ 2020-02-27 22:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: b.galvani, peter.maydell, Niek Linnenbank, qemu-arm, philmd

These patches change the Cubieboard machine definition to use the
correct CPU type, which is ARM Cortex-A8 instead of ARM Cortex-A9.

Additionally, add some sanity checks for the machine input
arguments in the initialization function.

Niek Linnenbank (4):
  hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine
    definition
  hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8
  hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB
  hw/arm/cubieboard: report error when using unsupported -bios argument

 hw/arm/cubieboard.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

-- 
2.17.1



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

* [PATCH v1 1/4] hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine definition
  2020-02-27 22:01 [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Niek Linnenbank
@ 2020-02-27 22:01 ` Niek Linnenbank
  2020-03-02 15:32   ` Peter Maydell
  2020-02-27 22:01 ` [PATCH v1 2/4] hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8 Niek Linnenbank
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Niek Linnenbank @ 2020-02-27 22:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: b.galvani, peter.maydell, Niek Linnenbank, qemu-arm, philmd

The Cubieboard is a singleboard computer with an Allwinner A10 System-on-Chip [1].
As documented in the Allwinner A10 User Manual V1.5 [2], the SoC has an ARM
Cortex-A8 processor. Currently the Cubieboard machine definition specifies the
ARM Cortex-A9 in its description and as the default CPU.

This patch corrects the Cubieboard machine definition to use the ARM Cortex-A8.

 [1] http://docs.cubieboard.org/products/start#cubieboard1
 [2] https://linux-sunxi.org/File:Allwinner_A10_User_manual_V1.5.pdf

Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
---
 hw/arm/cubieboard.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 089f9a30c1..0195925c73 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -68,8 +68,8 @@ static void cubieboard_init(MachineState *machine)
 
 static void cubieboard_machine_init(MachineClass *mc)
 {
-    mc->desc = "cubietech cubieboard (Cortex-A9)";
-    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
+    mc->desc = "cubietech cubieboard (Cortex-A8)";
+    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
     mc->init = cubieboard_init;
     mc->block_default_type = IF_IDE;
     mc->units_per_default_bus = 1;
-- 
2.17.1



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

* [PATCH v1 2/4] hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8
  2020-02-27 22:01 [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Niek Linnenbank
  2020-02-27 22:01 ` [PATCH v1 1/4] hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine definition Niek Linnenbank
@ 2020-02-27 22:01 ` Niek Linnenbank
  2020-03-02 15:33   ` Peter Maydell
  2020-02-27 22:01 ` [PATCH v1 3/4] hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB Niek Linnenbank
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Niek Linnenbank @ 2020-02-27 22:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: b.galvani, peter.maydell, Niek Linnenbank, qemu-arm, philmd

The Cubieboard has an ARM Cortex-A8. Prevent changing the CPU
to a different type which could break user programs.

Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
---
 hw/arm/cubieboard.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 0195925c73..010375f0a8 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -30,9 +30,17 @@ static struct arm_boot_info cubieboard_binfo = {
 
 static void cubieboard_init(MachineState *machine)
 {
-    AwA10State *a10 = AW_A10(object_new(TYPE_AW_A10));
+    AwA10State *a10;
     Error *err = NULL;
 
+    /* Only allow Cortex-A8 for this board */
+    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a8")) != 0) {
+        error_report("This board can only be used with cortex-a8 CPU");
+        exit(1);
+    }
+
+    a10 = AW_A10(object_new(TYPE_AW_A10));
+
     object_property_set_int(OBJECT(&a10->emac), 1, "phy-addr", &err);
     if (err != NULL) {
         error_reportf_err(err, "Couldn't set phy address: ");
-- 
2.17.1



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

* [PATCH v1 3/4] hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB
  2020-02-27 22:01 [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Niek Linnenbank
  2020-02-27 22:01 ` [PATCH v1 1/4] hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine definition Niek Linnenbank
  2020-02-27 22:01 ` [PATCH v1 2/4] hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8 Niek Linnenbank
@ 2020-02-27 22:01 ` Niek Linnenbank
  2020-03-02 15:41   ` Peter Maydell
  2020-02-27 22:01 ` [PATCH v1 4/4] hw/arm/cubieboard: report error when using unsupported -bios argument Niek Linnenbank
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Niek Linnenbank @ 2020-02-27 22:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: b.galvani, peter.maydell, Niek Linnenbank, qemu-arm, philmd

The Cubieboard contains either 512MiB or 1GiB of onboard RAM [1].
Prevent changing RAM to a different size which could break user programs.

 [1] http://linux-sunxi.org/Cubieboard

Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
---
 hw/arm/cubieboard.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 010375f0a8..6c55d9056f 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -33,6 +33,13 @@ static void cubieboard_init(MachineState *machine)
     AwA10State *a10;
     Error *err = NULL;
 
+    /* This board has fixed size RAM (512MiB or 1GiB) */
+    if (machine->ram_size != 512 * MiB &&
+        machine->ram_size != 1 * GiB) {
+        error_report("This machine can only be used with 512MiB or 1GiB RAM");
+        exit(1);
+    }
+
     /* Only allow Cortex-A8 for this board */
     if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a8")) != 0) {
         error_report("This board can only be used with cortex-a8 CPU");
@@ -78,6 +85,7 @@ static void cubieboard_machine_init(MachineClass *mc)
 {
     mc->desc = "cubietech cubieboard (Cortex-A8)";
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
+    mc->default_ram_size = 1 * GiB;
     mc->init = cubieboard_init;
     mc->block_default_type = IF_IDE;
     mc->units_per_default_bus = 1;
-- 
2.17.1



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

* [PATCH v1 4/4] hw/arm/cubieboard: report error when using unsupported -bios argument
  2020-02-27 22:01 [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Niek Linnenbank
                   ` (2 preceding siblings ...)
  2020-02-27 22:01 ` [PATCH v1 3/4] hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB Niek Linnenbank
@ 2020-02-27 22:01 ` Niek Linnenbank
  2020-03-02 15:44   ` Peter Maydell
  2020-03-02 16:06 ` [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Peter Maydell
  2020-03-02 18:05 ` Philippe Mathieu-Daudé
  5 siblings, 1 reply; 15+ messages in thread
From: Niek Linnenbank @ 2020-02-27 22:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: b.galvani, peter.maydell, Niek Linnenbank, qemu-arm, philmd

The Cubieboard machine does not support the -bios argument.
Report an error when -bios is used and exit immediately.

Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
---
 hw/arm/cubieboard.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 6c55d9056f..871b1beef4 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -19,6 +19,7 @@
 #include "exec/address-spaces.h"
 #include "qapi/error.h"
 #include "cpu.h"
+#include "sysemu/sysemu.h"
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/arm/allwinner-a10.h"
@@ -33,6 +34,12 @@ static void cubieboard_init(MachineState *machine)
     AwA10State *a10;
     Error *err = NULL;
 
+    /* BIOS is not supported by this board */
+    if (bios_name) {
+        error_report("BIOS not supported for this machine");
+        exit(1);
+    }
+
     /* This board has fixed size RAM (512MiB or 1GiB) */
     if (machine->ram_size != 512 * MiB &&
         machine->ram_size != 1 * GiB) {
-- 
2.17.1



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

* Re: [PATCH v1 1/4] hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine definition
  2020-02-27 22:01 ` [PATCH v1 1/4] hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine definition Niek Linnenbank
@ 2020-03-02 15:32   ` Peter Maydell
  2020-03-02 17:54     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2020-03-02 15:32 UTC (permalink / raw)
  To: Niek Linnenbank
  Cc: Beniamino Galvani, qemu-arm, Philippe Mathieu-Daudé,
	QEMU Developers

On Thu, 27 Feb 2020 at 22:01, Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
>
> The Cubieboard is a singleboard computer with an Allwinner A10 System-on-Chip [1].
> As documented in the Allwinner A10 User Manual V1.5 [2], the SoC has an ARM
> Cortex-A8 processor. Currently the Cubieboard machine definition specifies the
> ARM Cortex-A9 in its description and as the default CPU.
>
> This patch corrects the Cubieboard machine definition to use the ARM Cortex-A8.
>
>  [1] http://docs.cubieboard.org/products/start#cubieboard1
>  [2] https://linux-sunxi.org/File:Allwinner_A10_User_manual_V1.5.pdf
>
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> ---
>  hw/arm/cubieboard.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
> index 089f9a30c1..0195925c73 100644
> --- a/hw/arm/cubieboard.c
> +++ b/hw/arm/cubieboard.c
> @@ -68,8 +68,8 @@ static void cubieboard_init(MachineState *machine)
>
>  static void cubieboard_machine_init(MachineClass *mc)
>  {
> -    mc->desc = "cubietech cubieboard (Cortex-A9)";
> -    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
> +    mc->desc = "cubietech cubieboard (Cortex-A8)";
> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
>      mc->init = cubieboard_init;
>      mc->block_default_type = IF_IDE;
>      mc->units_per_default_bus = 1;

This is worth fixing, but I'm pretty sure it doesn't have
any user-visible effects, because the CPU is created by
hw/arm/allwinner-a10.c:aw_a10_init(), which always uses
cortex-a8 regardless of what the user specified on the command
line or what the mc->default_cpu_type is.

Fixes: 8a863c8120994981a099

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
if you tweak the commit message to mention that it
wasn't a user-visible bug (but I'll do that myself if there
isn't anything else in the series that means it needs a respin).

thanks
-- PMM


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

* Re: [PATCH v1 2/4] hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8
  2020-02-27 22:01 ` [PATCH v1 2/4] hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8 Niek Linnenbank
@ 2020-03-02 15:33   ` Peter Maydell
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2020-03-02 15:33 UTC (permalink / raw)
  To: Niek Linnenbank
  Cc: Beniamino Galvani, qemu-arm, Philippe Mathieu-Daudé,
	QEMU Developers

On Thu, 27 Feb 2020 at 22:01, Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
>
> The Cubieboard has an ARM Cortex-A8. Prevent changing the CPU
> to a different type which could break user programs.
>
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> ---
>  hw/arm/cubieboard.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
> index 0195925c73..010375f0a8 100644
> --- a/hw/arm/cubieboard.c
> +++ b/hw/arm/cubieboard.c
> @@ -30,9 +30,17 @@ static struct arm_boot_info cubieboard_binfo = {
>
>  static void cubieboard_init(MachineState *machine)
>  {
> -    AwA10State *a10 = AW_A10(object_new(TYPE_AW_A10));
> +    AwA10State *a10;
>      Error *err = NULL;
>
> +    /* Only allow Cortex-A8 for this board */
> +    if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a8")) != 0) {
> +        error_report("This board can only be used with cortex-a8 CPU");
> +        exit(1);
> +    }
> +
> +    a10 = AW_A10(object_new(TYPE_AW_A10));
> +
>      object_property_set_int(OBJECT(&a10->emac), 1, "phy-addr", &err);
>      if (err != NULL) {
>          error_reportf_err(err, "Couldn't set phy address: ");

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

except that we're not preventing the user changing the
CPU type, we're just giving a helpful error message instead
of ignoring the bogus -cpu option.

thanks
-- PMM


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

* Re: [PATCH v1 3/4] hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB
  2020-02-27 22:01 ` [PATCH v1 3/4] hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB Niek Linnenbank
@ 2020-03-02 15:41   ` Peter Maydell
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2020-03-02 15:41 UTC (permalink / raw)
  To: Niek Linnenbank
  Cc: Beniamino Galvani, qemu-arm, Philippe Mathieu-Daudé,
	QEMU Developers

On Thu, 27 Feb 2020 at 22:02, Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
>
> The Cubieboard contains either 512MiB or 1GiB of onboard RAM [1].
> Prevent changing RAM to a different size which could break user programs.
>
>  [1] http://linux-sunxi.org/Cubieboard
>
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


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

* Re: [PATCH v1 4/4] hw/arm/cubieboard: report error when using unsupported -bios argument
  2020-02-27 22:01 ` [PATCH v1 4/4] hw/arm/cubieboard: report error when using unsupported -bios argument Niek Linnenbank
@ 2020-03-02 15:44   ` Peter Maydell
  2020-03-02 18:04     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2020-03-02 15:44 UTC (permalink / raw)
  To: Niek Linnenbank
  Cc: Beniamino Galvani, qemu-arm, Philippe Mathieu-Daudé,
	QEMU Developers

On Thu, 27 Feb 2020 at 22:02, Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
>
> The Cubieboard machine does not support the -bios argument.
> Report an error when -bios is used and exit immediately.
>
> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> ---
>  hw/arm/cubieboard.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
> index 6c55d9056f..871b1beef4 100644
> --- a/hw/arm/cubieboard.c
> +++ b/hw/arm/cubieboard.c
> @@ -19,6 +19,7 @@
>  #include "exec/address-spaces.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
> +#include "sysemu/sysemu.h"
>  #include "hw/sysbus.h"
>  #include "hw/boards.h"
>  #include "hw/arm/allwinner-a10.h"
> @@ -33,6 +34,12 @@ static void cubieboard_init(MachineState *machine)
>      AwA10State *a10;
>      Error *err = NULL;
>
> +    /* BIOS is not supported by this board */
> +    if (bios_name) {
> +        error_report("BIOS not supported for this machine");
> +        exit(1);
> +    }

We don't usually bother to check this, but I guess there's
no reason not to.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


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

* Re: [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks
  2020-02-27 22:01 [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Niek Linnenbank
                   ` (3 preceding siblings ...)
  2020-02-27 22:01 ` [PATCH v1 4/4] hw/arm/cubieboard: report error when using unsupported -bios argument Niek Linnenbank
@ 2020-03-02 16:06 ` Peter Maydell
  2020-03-02 22:21   ` Niek Linnenbank
  2020-03-02 18:05 ` Philippe Mathieu-Daudé
  5 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2020-03-02 16:06 UTC (permalink / raw)
  To: Niek Linnenbank
  Cc: Beniamino Galvani, qemu-arm, Philippe Mathieu-Daudé,
	QEMU Developers

On Thu, 27 Feb 2020 at 22:01, Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
>
> These patches change the Cubieboard machine definition to use the
> correct CPU type, which is ARM Cortex-A8 instead of ARM Cortex-A9.
>
> Additionally, add some sanity checks for the machine input
> arguments in the initialization function.
>
> Niek Linnenbank (4):
>   hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine
>     definition
>   hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8
>   hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB
>   hw/arm/cubieboard: report error when using unsupported -bios argument


Applied to target-arm.next, thanks. (I tweaked a couple of commit
messages as mentioned in replies to those patches.)

-- PMM


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

* Re: [PATCH v1 1/4] hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine definition
  2020-03-02 15:32   ` Peter Maydell
@ 2020-03-02 17:54     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-02 17:54 UTC (permalink / raw)
  To: Peter Maydell, Niek Linnenbank
  Cc: Beniamino Galvani, qemu-arm, QEMU Developers

On 3/2/20 4:32 PM, Peter Maydell wrote:
> On Thu, 27 Feb 2020 at 22:01, Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
>>
>> The Cubieboard is a singleboard computer with an Allwinner A10 System-on-Chip [1].
>> As documented in the Allwinner A10 User Manual V1.5 [2], the SoC has an ARM
>> Cortex-A8 processor. Currently the Cubieboard machine definition specifies the
>> ARM Cortex-A9 in its description and as the default CPU.
>>
>> This patch corrects the Cubieboard machine definition to use the ARM Cortex-A8.
>>
>>   [1] http://docs.cubieboard.org/products/start#cubieboard1
>>   [2] https://linux-sunxi.org/File:Allwinner_A10_User_manual_V1.5.pdf
>>
>> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>> ---
>>   hw/arm/cubieboard.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
>> index 089f9a30c1..0195925c73 100644
>> --- a/hw/arm/cubieboard.c
>> +++ b/hw/arm/cubieboard.c
>> @@ -68,8 +68,8 @@ static void cubieboard_init(MachineState *machine)
>>
>>   static void cubieboard_machine_init(MachineClass *mc)
>>   {
>> -    mc->desc = "cubietech cubieboard (Cortex-A9)";
>> -    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9");
>> +    mc->desc = "cubietech cubieboard (Cortex-A8)";
>> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a8");
>>       mc->init = cubieboard_init;
>>       mc->block_default_type = IF_IDE;
>>       mc->units_per_default_bus = 1;
> 
> This is worth fixing, but I'm pretty sure it doesn't have
> any user-visible effects, because the CPU is created by
> hw/arm/allwinner-a10.c:aw_a10_init(), which always uses
> cortex-a8 regardless of what the user specified on the command
> line or what the mc->default_cpu_type is.

It is worth fixing if we continue the MachineClass code cleanup Igor 
wants (moving the ram size / cpu type checks in the machine common code).

> 
> Fixes: 8a863c8120994981a099
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> if you tweak the commit message to mention that it
> wasn't a user-visible bug (but I'll do that myself if there
> isn't anything else in the series that means it needs a respin).
> 
> thanks
> -- PMM
> 



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

* Re: [PATCH v1 4/4] hw/arm/cubieboard: report error when using unsupported -bios argument
  2020-03-02 15:44   ` Peter Maydell
@ 2020-03-02 18:04     ` Philippe Mathieu-Daudé
  2020-03-02 22:30       ` Niek Linnenbank
  0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-02 18:04 UTC (permalink / raw)
  To: Peter Maydell, Niek Linnenbank
  Cc: Beniamino Galvani, qemu-arm, QEMU Developers

On 3/2/20 4:44 PM, Peter Maydell wrote:
> On Thu, 27 Feb 2020 at 22:02, Niek Linnenbank <nieklinnenbank@gmail.com> wrote:
>>
>> The Cubieboard machine does not support the -bios argument.
>> Report an error when -bios is used and exit immediately.
>>
>> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>> ---
>>   hw/arm/cubieboard.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
>> index 6c55d9056f..871b1beef4 100644
>> --- a/hw/arm/cubieboard.c
>> +++ b/hw/arm/cubieboard.c
>> @@ -19,6 +19,7 @@
>>   #include "exec/address-spaces.h"
>>   #include "qapi/error.h"
>>   #include "cpu.h"
>> +#include "sysemu/sysemu.h"
>>   #include "hw/sysbus.h"
>>   #include "hw/boards.h"
>>   #include "hw/arm/allwinner-a10.h"
>> @@ -33,6 +34,12 @@ static void cubieboard_init(MachineState *machine)
>>       AwA10State *a10;
>>       Error *err = NULL;
>>
>> +    /* BIOS is not supported by this board */
>> +    if (bios_name) {
>> +        error_report("BIOS not supported for this machine");
>> +        exit(1);
>> +    }
> 
> We don't usually bother to check this, but I guess there's
> no reason not to.

I agree this is confusing to expect the machine boot from a flash when 
using -bios and having to debug until figuring out the reason.

This -bios is a generic machine option, maybe we could move this check 
to the common machine code.

> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM
> 



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

* Re: [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks
  2020-02-27 22:01 [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Niek Linnenbank
                   ` (4 preceding siblings ...)
  2020-03-02 16:06 ` [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Peter Maydell
@ 2020-03-02 18:05 ` Philippe Mathieu-Daudé
  5 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-02 18:05 UTC (permalink / raw)
  To: Niek Linnenbank, qemu-devel; +Cc: b.galvani, peter.maydell, qemu-arm

On 2/27/20 11:01 PM, Niek Linnenbank wrote:
> These patches change the Cubieboard machine definition to use the
> correct CPU type, which is ARM Cortex-A8 instead of ARM Cortex-A9.
> 
> Additionally, add some sanity checks for the machine input
> arguments in the initialization function.
> 
> Niek Linnenbank (4):
>    hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine
>      definition
>    hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8
>    hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB
>    hw/arm/cubieboard: report error when using unsupported -bios argument
> 
>   hw/arm/cubieboard.c | 29 ++++++++++++++++++++++++++---
>   1 file changed, 26 insertions(+), 3 deletions(-)
> 

Series:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



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

* Re: [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks
  2020-03-02 16:06 ` [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Peter Maydell
@ 2020-03-02 22:21   ` Niek Linnenbank
  0 siblings, 0 replies; 15+ messages in thread
From: Niek Linnenbank @ 2020-03-02 22:21 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Beniamino Galvani, qemu-arm, Philippe Mathieu-Daudé,
	QEMU Developers

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

Hi Peter, Philippe,

On Mon, Mar 2, 2020 at 5:06 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Thu, 27 Feb 2020 at 22:01, Niek Linnenbank <nieklinnenbank@gmail.com>
> wrote:
> >
> > These patches change the Cubieboard machine definition to use the
> > correct CPU type, which is ARM Cortex-A8 instead of ARM Cortex-A9.
> >
> > Additionally, add some sanity checks for the machine input
> > arguments in the initialization function.
> >
> > Niek Linnenbank (4):
> >   hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine
> >     definition
> >   hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8
> >   hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB
> >   hw/arm/cubieboard: report error when using unsupported -bios argument
>
>
> Applied to target-arm.next, thanks. (I tweaked a couple of commit
> messages as mentioned in replies to those patches.)
>

Great, and thanks for your quick and detailed responses Peter.

And to be clear, these improvements came from the excellent reviews done by
Philippe on the Allwinner H3 patch series
which I submitted earlier. Since that series is quite big and will take
more time to complete, I thought it would be nice
to take a few of those improvements and meanwhile apply them to the other
similar boards as well.

Regards,
Niek

>
> -- PMM
>


-- 
Niek Linnenbank

[-- Attachment #2: Type: text/html, Size: 2182 bytes --]

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

* Re: [PATCH v1 4/4] hw/arm/cubieboard: report error when using unsupported -bios argument
  2020-03-02 18:04     ` Philippe Mathieu-Daudé
@ 2020-03-02 22:30       ` Niek Linnenbank
  0 siblings, 0 replies; 15+ messages in thread
From: Niek Linnenbank @ 2020-03-02 22:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Beniamino Galvani, Peter Maydell, qemu-arm, QEMU Developers

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

Hi Philippe,

On Mon, Mar 2, 2020 at 7:04 PM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> On 3/2/20 4:44 PM, Peter Maydell wrote:
> > On Thu, 27 Feb 2020 at 22:02, Niek Linnenbank <nieklinnenbank@gmail.com>
> wrote:
> >>
> >> The Cubieboard machine does not support the -bios argument.
> >> Report an error when -bios is used and exit immediately.
> >>
> >> Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> >> ---
> >>   hw/arm/cubieboard.c | 7 +++++++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
> >> index 6c55d9056f..871b1beef4 100644
> >> --- a/hw/arm/cubieboard.c
> >> +++ b/hw/arm/cubieboard.c
> >> @@ -19,6 +19,7 @@
> >>   #include "exec/address-spaces.h"
> >>   #include "qapi/error.h"
> >>   #include "cpu.h"
> >> +#include "sysemu/sysemu.h"
> >>   #include "hw/sysbus.h"
> >>   #include "hw/boards.h"
> >>   #include "hw/arm/allwinner-a10.h"
> >> @@ -33,6 +34,12 @@ static void cubieboard_init(MachineState *machine)
> >>       AwA10State *a10;
> >>       Error *err = NULL;
> >>
> >> +    /* BIOS is not supported by this board */
> >> +    if (bios_name) {
> >> +        error_report("BIOS not supported for this machine");
> >> +        exit(1);
> >> +    }
> >
> > We don't usually bother to check this, but I guess there's
> > no reason not to.
>
> I agree this is confusing to expect the machine boot from a flash when
> using -bios and having to debug until figuring out the reason.
>
> This -bios is a generic machine option, maybe we could move this check
> to the common machine code.
>

Agreed, that sounds logical indeed.

When we have a common machine code/class I presume that boards which do
support the -bios
option could simply set somekind of flag/field to enable the -bios
functionality. And for boards
which do not support it, the common machine code would then by default give
an error and exit.

Regards,
Niek


> >
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> >
> > thanks
> > -- PMM
> >
>
>

-- 
Niek Linnenbank

[-- Attachment #2: Type: text/html, Size: 3397 bytes --]

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

end of thread, other threads:[~2020-03-02 22:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 22:01 [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Niek Linnenbank
2020-02-27 22:01 ` [PATCH v1 1/4] hw/arm/cubieboard: use ARM Cortex-A8 as the default CPU in machine definition Niek Linnenbank
2020-03-02 15:32   ` Peter Maydell
2020-03-02 17:54     ` Philippe Mathieu-Daudé
2020-02-27 22:01 ` [PATCH v1 2/4] hw/arm/cubieboard: restrict allowed CPU type to ARM Cortex-A8 Niek Linnenbank
2020-03-02 15:33   ` Peter Maydell
2020-02-27 22:01 ` [PATCH v1 3/4] hw/arm/cubieboard: restrict allowed RAM size to 512MiB and 1GiB Niek Linnenbank
2020-03-02 15:41   ` Peter Maydell
2020-02-27 22:01 ` [PATCH v1 4/4] hw/arm/cubieboard: report error when using unsupported -bios argument Niek Linnenbank
2020-03-02 15:44   ` Peter Maydell
2020-03-02 18:04     ` Philippe Mathieu-Daudé
2020-03-02 22:30       ` Niek Linnenbank
2020-03-02 16:06 ` [PATCH v1 0/4] hw/arm/cubieboard: correct CPU type and add machine argument checks Peter Maydell
2020-03-02 22:21   ` Niek Linnenbank
2020-03-02 18:05 ` Philippe Mathieu-Daudé

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.