diff mbox series

[v1,4/4] hw/arm/cubieboard: report error when using unsupported -bios argument

Message ID 20200227220149.6845-5-nieklinnenbank@gmail.com (mailing list archive)
State New, archived
Headers show
Series hw/arm/cubieboard: correct CPU type and add machine argument checks | expand

Commit Message

Niek Linnenbank Feb. 27, 2020, 10:01 p.m. UTC
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(+)

Comments

Peter Maydell March 2, 2020, 3:44 p.m. UTC | #1
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
Philippe Mathieu-Daudé March 2, 2020, 6:04 p.m. UTC | #2
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
>
Niek Linnenbank March 2, 2020, 10:30 p.m. UTC | #3
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
> >
>
>
diff mbox series

Patch

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) {