hw/riscv: do not mark any machine as default

[PATCH for-10.1] hw/riscv: do not mark any machine as default

Posted by Daniel Henrique Barboza 10 months, 2 weeks ago

Commit 5b4beba124 ("RISC-V Spike Machines") added the Spike machine and
made it default for qemu-system-riscv32/64. It was the first RISC-V
machine added in QEMU so setting it as default was sensible.

Today we have 7 risc64 and 6 riscv32 machines and having 'spike' as
default machine is not intuitive. For example, [1] is a bug that was
opened with the 'virt' board in mind, but given that the user didn't
pass a '-machine' option, the user was using 'spike' without knowing.

The QEMU archs that defines a default machine usually defines it as the
most used machine, e.g. PowerPC uses 'pseries' as default. So in theory
we could change the default to the 'virt' machine, but that would make
existing command lines that don't specify a machine option to act
weird: they would silently use 'virt' instead of 'spike'.

Being explicit in the command line is desirable when we have a handful
of boards available, so remove the default machine setting from RISC-V
and make it obligatory to specify the board.

After this patch we'll throw an error if no machine is specified:

$ ./build/qemu-system-riscv64 --nographic qemu-system-riscv64: No
machine specified, and there is no default Use -machine help to list
supported machines

'spike' users that aren't specifying their machines in the command line
will be impacted and will need to add '-M spike' in their scripts.

[1] https://gitlab.com/qemu-project/qemu/-/issues/2467

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/spike.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 74a20016f1..ba88d3a07b 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -349,7 +349,6 @@ static void spike_machine_class_init(ObjectClass *oc, void *data)
     mc->desc = "RISC-V Spike board";
     mc->init = spike_board_init;
     mc->max_cpus = SPIKE_CPUS_MAX;
-    mc->is_default = true;
     mc->default_cpu_type = TYPE_RISCV_CPU_BASE;
     mc->possible_cpu_arch_ids = riscv_numa_possible_cpu_arch_ids;
     mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
-- 
2.48.1

Re: [PATCH for-10.1] hw/riscv: do not mark any machine as default

Posted by Philippe Mathieu-Daudé 10 months, 2 weeks ago

On 27/3/25 14:02, Daniel Henrique Barboza wrote:
> Commit 5b4beba124 ("RISC-V Spike Machines") added the Spike machine and
> made it default for qemu-system-riscv32/64. It was the first RISC-V
> machine added in QEMU so setting it as default was sensible.
> 
> Today we have 7 risc64 and 6 riscv32 machines and having 'spike' as
> default machine is not intuitive. For example, [1] is a bug that was
> opened with the 'virt' board in mind, but given that the user didn't
> pass a '-machine' option, the user was using 'spike' without knowing.
> 
> The QEMU archs that defines a default machine usually defines it as the
> most used machine, e.g. PowerPC uses 'pseries' as default. So in theory
> we could change the default to the 'virt' machine, but that would make
> existing command lines that don't specify a machine option to act
> weird: they would silently use 'virt' instead of 'spike'.
> 
> Being explicit in the command line is desirable when we have a handful
> of boards available, so remove the default machine setting from RISC-V
> and make it obligatory to specify the board.
> 
> After this patch we'll throw an error if no machine is specified:
> 
> $ ./build/qemu-system-riscv64 --nographic qemu-system-riscv64: No
> machine specified, and there is no default Use -machine help to list
> supported machines
> 
> 'spike' users that aren't specifying their machines in the command line
> will be impacted and will need to add '-M spike' in their scripts.
> 
> [1] https://gitlab.com/qemu-project/qemu/-/issues/2467
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   hw/riscv/spike.c | 1 -
>   1 file changed, 1 deletion(-)

I'm in favor of this change, which I believe is the correct way to
go, so:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
but I'd rather we follow the deprecation process. Up to the maintainer.

Re: [PATCH for-10.1] hw/riscv: do not mark any machine as default

Posted by Alistair Francis 10 months, 1 week ago

On Fri, Mar 28, 2025 at 2:16 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 27/3/25 14:02, Daniel Henrique Barboza wrote:
> > Commit 5b4beba124 ("RISC-V Spike Machines") added the Spike machine and
> > made it default for qemu-system-riscv32/64. It was the first RISC-V
> > machine added in QEMU so setting it as default was sensible.
> >
> > Today we have 7 risc64 and 6 riscv32 machines and having 'spike' as
> > default machine is not intuitive. For example, [1] is a bug that was
> > opened with the 'virt' board in mind, but given that the user didn't
> > pass a '-machine' option, the user was using 'spike' without knowing.
> >
> > The QEMU archs that defines a default machine usually defines it as the
> > most used machine, e.g. PowerPC uses 'pseries' as default. So in theory
> > we could change the default to the 'virt' machine, but that would make
> > existing command lines that don't specify a machine option to act
> > weird: they would silently use 'virt' instead of 'spike'.
> >
> > Being explicit in the command line is desirable when we have a handful
> > of boards available, so remove the default machine setting from RISC-V
> > and make it obligatory to specify the board.
> >
> > After this patch we'll throw an error if no machine is specified:
> >
> > $ ./build/qemu-system-riscv64 --nographic qemu-system-riscv64: No
> > machine specified, and there is no default Use -machine help to list
> > supported machines
> >
> > 'spike' users that aren't specifying their machines in the command line
> > will be impacted and will need to add '-M spike' in their scripts.
> >
> > [1] https://gitlab.com/qemu-project/qemu/-/issues/2467
> >
> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > ---
> >   hw/riscv/spike.c | 1 -
> >   1 file changed, 1 deletion(-)
>
> I'm in favor of this change, which I believe is the correct way to
> go, so:

Agreed

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> but I'd rather we follow the deprecation process. Up to the maintainer.

I agree, it is a breaking change, it would be nice to go through the
deprecation process in case people are expecting Spike to be the
default.

Alistair

>
>
>

Re: [PATCH for-10.1] hw/riscv: do not mark any machine as default

Posted by Daniel Henrique Barboza 10 months, 1 week ago


On 4/4/25 2:50 AM, Alistair Francis wrote:
> On Fri, Mar 28, 2025 at 2:16 AM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
>>
>> On 27/3/25 14:02, Daniel Henrique Barboza wrote:
>>> Commit 5b4beba124 ("RISC-V Spike Machines") added the Spike machine and
>>> made it default for qemu-system-riscv32/64. It was the first RISC-V
>>> machine added in QEMU so setting it as default was sensible.
>>>
>>> Today we have 7 risc64 and 6 riscv32 machines and having 'spike' as
>>> default machine is not intuitive. For example, [1] is a bug that was
>>> opened with the 'virt' board in mind, but given that the user didn't
>>> pass a '-machine' option, the user was using 'spike' without knowing.
>>>
>>> The QEMU archs that defines a default machine usually defines it as the
>>> most used machine, e.g. PowerPC uses 'pseries' as default. So in theory
>>> we could change the default to the 'virt' machine, but that would make
>>> existing command lines that don't specify a machine option to act
>>> weird: they would silently use 'virt' instead of 'spike'.
>>>
>>> Being explicit in the command line is desirable when we have a handful
>>> of boards available, so remove the default machine setting from RISC-V
>>> and make it obligatory to specify the board.
>>>
>>> After this patch we'll throw an error if no machine is specified:
>>>
>>> $ ./build/qemu-system-riscv64 --nographic qemu-system-riscv64: No
>>> machine specified, and there is no default Use -machine help to list
>>> supported machines
>>>
>>> 'spike' users that aren't specifying their machines in the command line
>>> will be impacted and will need to add '-M spike' in their scripts.
>>>
>>> [1] https://gitlab.com/qemu-project/qemu/-/issues/2467
>>>
>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>> ---
>>>    hw/riscv/spike.c | 1 -
>>>    1 file changed, 1 deletion(-)
>>
>> I'm in favor of this change, which I believe is the correct way to
>> go, so:
> 
> Agreed
> 
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> but I'd rather we follow the deprecation process. Up to the maintainer.
> 
> I agree, it is a breaking change, it would be nice to go through the
> deprecation process in case people are expecting Spike to be the
> default.

I don't mind going through the deprecation process in this case since we're
not just eliminating a default value, we're removing it.

What about other default val related changes, e.g. do we have to go through\
the deprecation process to change the default CPU of a board? And yeah,
spoiler alert :D


Thanks,

Daniel

> 
> Alistair
> 
>>
>>
>>

Re: [PATCH for-10.1] hw/riscv: do not mark any machine as default

Posted by Philippe Mathieu-Daudé 10 months, 1 week ago

On 4/4/25 13:30, Daniel Henrique Barboza wrote:
> 
> 
> On 4/4/25 2:50 AM, Alistair Francis wrote:
>> On Fri, Mar 28, 2025 at 2:16 AM Philippe Mathieu-Daudé
>> <philmd@linaro.org> wrote:
>>>
>>> On 27/3/25 14:02, Daniel Henrique Barboza wrote:
>>>> Commit 5b4beba124 ("RISC-V Spike Machines") added the Spike machine and
>>>> made it default for qemu-system-riscv32/64. It was the first RISC-V
>>>> machine added in QEMU so setting it as default was sensible.
>>>>
>>>> Today we have 7 risc64 and 6 riscv32 machines and having 'spike' as
>>>> default machine is not intuitive. For example, [1] is a bug that was
>>>> opened with the 'virt' board in mind, but given that the user didn't
>>>> pass a '-machine' option, the user was using 'spike' without knowing.
>>>>
>>>> The QEMU archs that defines a default machine usually defines it as the
>>>> most used machine, e.g. PowerPC uses 'pseries' as default. So in theory
>>>> we could change the default to the 'virt' machine, but that would make
>>>> existing command lines that don't specify a machine option to act
>>>> weird: they would silently use 'virt' instead of 'spike'.
>>>>
>>>> Being explicit in the command line is desirable when we have a handful
>>>> of boards available, so remove the default machine setting from RISC-V
>>>> and make it obligatory to specify the board.
>>>>
>>>> After this patch we'll throw an error if no machine is specified:
>>>>
>>>> $ ./build/qemu-system-riscv64 --nographic qemu-system-riscv64: No
>>>> machine specified, and there is no default Use -machine help to list
>>>> supported machines
>>>>
>>>> 'spike' users that aren't specifying their machines in the command line
>>>> will be impacted and will need to add '-M spike' in their scripts.
>>>>
>>>> [1] https://gitlab.com/qemu-project/qemu/-/issues/2467
>>>>
>>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>>>> ---
>>>>    hw/riscv/spike.c | 1 -
>>>>    1 file changed, 1 deletion(-)
>>>
>>> I'm in favor of this change, which I believe is the correct way to
>>> go, so:
>>
>> Agreed
>>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> but I'd rather we follow the deprecation process. Up to the maintainer.
>>
>> I agree, it is a breaking change, it would be nice to go through the
>> deprecation process in case people are expecting Spike to be the
>> default.
> 
> I don't mind going through the deprecation process in this case since we're
> not just eliminating a default value, we're removing it.
> 
> What about other default val related changes, e.g. do we have to go 
> through\
> the deprecation process to change the default CPU of a board? And yeah,
> spoiler alert :D

I think so. My preference to avoid this is to remove default values ;)

Re: [PATCH for-10.1] hw/riscv: do not mark any machine as default

Posted by Andrew Jones 10 months, 1 week ago

On Fri, Apr 04, 2025 at 02:37:32PM +0200, Philippe Mathieu-Daudé wrote:
> On 4/4/25 13:30, Daniel Henrique Barboza wrote:
> > 
> > 
> > On 4/4/25 2:50 AM, Alistair Francis wrote:
> > > On Fri, Mar 28, 2025 at 2:16 AM Philippe Mathieu-Daudé
> > > <philmd@linaro.org> wrote:
> > > > 
> > > > On 27/3/25 14:02, Daniel Henrique Barboza wrote:
> > > > > Commit 5b4beba124 ("RISC-V Spike Machines") added the Spike machine and
> > > > > made it default for qemu-system-riscv32/64. It was the first RISC-V
> > > > > machine added in QEMU so setting it as default was sensible.
> > > > > 
> > > > > Today we have 7 risc64 and 6 riscv32 machines and having 'spike' as
> > > > > default machine is not intuitive. For example, [1] is a bug that was
> > > > > opened with the 'virt' board in mind, but given that the user didn't
> > > > > pass a '-machine' option, the user was using 'spike' without knowing.
> > > > > 
> > > > > The QEMU archs that defines a default machine usually defines it as the
> > > > > most used machine, e.g. PowerPC uses 'pseries' as default. So in theory
> > > > > we could change the default to the 'virt' machine, but that would make
> > > > > existing command lines that don't specify a machine option to act
> > > > > weird: they would silently use 'virt' instead of 'spike'.
> > > > > 
> > > > > Being explicit in the command line is desirable when we have a handful
> > > > > of boards available, so remove the default machine setting from RISC-V
> > > > > and make it obligatory to specify the board.
> > > > > 
> > > > > After this patch we'll throw an error if no machine is specified:
> > > > > 
> > > > > $ ./build/qemu-system-riscv64 --nographic qemu-system-riscv64: No
> > > > > machine specified, and there is no default Use -machine help to list
> > > > > supported machines
> > > > > 
> > > > > 'spike' users that aren't specifying their machines in the command line
> > > > > will be impacted and will need to add '-M spike' in their scripts.
> > > > > 
> > > > > [1] https://gitlab.com/qemu-project/qemu/-/issues/2467
> > > > > 
> > > > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> > > > > ---
> > > > >    hw/riscv/spike.c | 1 -
> > > > >    1 file changed, 1 deletion(-)
> > > > 
> > > > I'm in favor of this change, which I believe is the correct way to
> > > > go, so:
> > > 
> > > Agreed
> > > 
> > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > but I'd rather we follow the deprecation process. Up to the maintainer.
> > > 
> > > I agree, it is a breaking change, it would be nice to go through the
> > > deprecation process in case people are expecting Spike to be the
> > > default.
> > 
> > I don't mind going through the deprecation process in this case since we're
> > not just eliminating a default value, we're removing it.
> > 
> > What about other default val related changes, e.g. do we have to go
> > through\
> > the deprecation process to change the default CPU of a board? And yeah,
> > spoiler alert :D
> 

I don't think we need the deprecation process to change a board's default
cpu type from a generic type which is a strict subset of the type to which
we change (rv64 -> max). While changing to a superset may change behavior
(things that didn't work before will suddenly start working) generic cpu
types have always had the freedom to add new extensions. If we want to
avoid all behavior changes, then we'll need to start versioning the
generic cpu types (which might not be such a bad idea).

> I think so. My preference to avoid this is to remove default values ;)
>

I personally don't mind not having a default cpu type for the virt board,
but, if we want one, then I think the max cpu type makes the most sense
(like arm64 has).

Thanks,
drew

Re: [PATCH for-10.1] hw/riscv: do not mark any machine as default

Posted by Philippe Mathieu-Daudé 10 months, 2 weeks ago

On 27/3/25 17:14, Philippe Mathieu-Daudé wrote:
> On 27/3/25 14:02, Daniel Henrique Barboza wrote:
>> Commit 5b4beba124 ("RISC-V Spike Machines") added the Spike machine and
>> made it default for qemu-system-riscv32/64. It was the first RISC-V
>> machine added in QEMU so setting it as default was sensible.
>>
>> Today we have 7 risc64 and 6 riscv32 machines and having 'spike' as
>> default machine is not intuitive. For example, [1] is a bug that was
>> opened with the 'virt' board in mind, but given that the user didn't
>> pass a '-machine' option, the user was using 'spike' without knowing.
>>
>> The QEMU archs that defines a default machine usually defines it as the
>> most used machine, e.g. PowerPC uses 'pseries' as default. So in theory
>> we could change the default to the 'virt' machine, but that would make
>> existing command lines that don't specify a machine option to act
>> weird: they would silently use 'virt' instead of 'spike'.
>>
>> Being explicit in the command line is desirable when we have a handful
>> of boards available, so remove the default machine setting from RISC-V
>> and make it obligatory to specify the board.
>>
>> After this patch we'll throw an error if no machine is specified:
>>
>> $ ./build/qemu-system-riscv64 --nographic qemu-system-riscv64: No
>> machine specified, and there is no default Use -machine help to list
>> supported machines
>>
>> 'spike' users that aren't specifying their machines in the command line
>> will be impacted and will need to add '-M spike' in their scripts.
>>
>> [1] https://gitlab.com/qemu-project/qemu/-/issues/2467
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   hw/riscv/spike.c | 1 -
>>   1 file changed, 1 deletion(-)
> 
> I'm in favor of this change, which I believe is the correct way to
> go, so:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> but I'd rather we follow the deprecation process. Up to the maintainer.

And if we deprecate, the deprecation patch is OK to be merged for 10.0.

Re: [PATCH for-10.1] hw/riscv: do not mark any machine as default

Posted by Frank Chang 10 months, 2 weeks ago

Reviewed-by: Frank Chang <frank.chang@sifive.com>

Daniel Henrique Barboza <dbarboza@ventanamicro.com> 於 2025年3月27日 週四 下午9:04寫道：
>
> Commit 5b4beba124 ("RISC-V Spike Machines") added the Spike machine and
> made it default for qemu-system-riscv32/64. It was the first RISC-V
> machine added in QEMU so setting it as default was sensible.
>
> Today we have 7 risc64 and 6 riscv32 machines and having 'spike' as
> default machine is not intuitive. For example, [1] is a bug that was
> opened with the 'virt' board in mind, but given that the user didn't
> pass a '-machine' option, the user was using 'spike' without knowing.
>
> The QEMU archs that defines a default machine usually defines it as the
> most used machine, e.g. PowerPC uses 'pseries' as default. So in theory
> we could change the default to the 'virt' machine, but that would make
> existing command lines that don't specify a machine option to act
> weird: they would silently use 'virt' instead of 'spike'.
>
> Being explicit in the command line is desirable when we have a handful
> of boards available, so remove the default machine setting from RISC-V
> and make it obligatory to specify the board.
>
> After this patch we'll throw an error if no machine is specified:
>
> $ ./build/qemu-system-riscv64 --nographic qemu-system-riscv64: No
> machine specified, and there is no default Use -machine help to list
> supported machines
>
> 'spike' users that aren't specifying their machines in the command line
> will be impacted and will need to add '-M spike' in their scripts.
>
> [1] https://gitlab.com/qemu-project/qemu/-/issues/2467
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  hw/riscv/spike.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 74a20016f1..ba88d3a07b 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -349,7 +349,6 @@ static void spike_machine_class_init(ObjectClass *oc, void *data)
>      mc->desc = "RISC-V Spike board";
>      mc->init = spike_board_init;
>      mc->max_cpus = SPIKE_CPUS_MAX;
> -    mc->is_default = true;
>      mc->default_cpu_type = TYPE_RISCV_CPU_BASE;
>      mc->possible_cpu_arch_ids = riscv_numa_possible_cpu_arch_ids;
>      mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
> --
> 2.48.1
>
>