target/riscv/cpu.c: do not run 'host' CPU with TCG

[PATCH] target/riscv/cpu.c: do not run 'host' CPU with TCG

Posted by Daniel Henrique Barboza 9 months, 2 weeks ago

The 'host' CPU is available in a CONFIG_KVM build and it's currently
available for all accels, but is a KVM only CPU. This means that in a
RISC-V KVM capable host we can do things like this:

$ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
qemu-system-riscv64: H extension requires priv spec 1.12.0

This CPU does not have a priv spec because we don't filter its extensions
via priv spec. We shouldn't be reaching riscv_cpu_realize_tcg() at all
with the 'host' CPU.

We don't have a way to filter the 'host' CPU out of the available CPU
options (-cpu help) if the build includes both KVM and TCG. What we can
do is to error out during riscv_cpu_realize_tcg() if the user chooses
the 'host' CPU with accel=tcg:

$ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
qemu-system-riscv64: 'host' CPU is not compatible with TCG acceleration

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6b93b04453..08db3d613f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1395,6 +1395,11 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)
     CPURISCVState *env = &cpu->env;
     Error *local_err = NULL;
 
+    if (object_dynamic_cast(OBJECT(dev), TYPE_RISCV_CPU_HOST)) {
+        error_setg(errp, "'host' CPU is not compatible with TCG acceleration");
+        return;
+    }
+
     riscv_cpu_validate_misa_mxl(cpu, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
-- 
2.41.0

Re: [PATCH] target/riscv/cpu.c: do not run 'host' CPU with TCG

Posted by Alistair Francis 9 months, 1 week ago

On Fri, Jul 21, 2023 at 11:35 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The 'host' CPU is available in a CONFIG_KVM build and it's currently
> available for all accels, but is a KVM only CPU. This means that in a
> RISC-V KVM capable host we can do things like this:
>
> $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
> qemu-system-riscv64: H extension requires priv spec 1.12.0
>
> This CPU does not have a priv spec because we don't filter its extensions
> via priv spec. We shouldn't be reaching riscv_cpu_realize_tcg() at all
> with the 'host' CPU.
>
> We don't have a way to filter the 'host' CPU out of the available CPU
> options (-cpu help) if the build includes both KVM and TCG. What we can
> do is to error out during riscv_cpu_realize_tcg() if the user chooses
> the 'host' CPU with accel=tcg:
>
> $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
> qemu-system-riscv64: 'host' CPU is not compatible with TCG acceleration
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/cpu.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 6b93b04453..08db3d613f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1395,6 +1395,11 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)
>      CPURISCVState *env = &cpu->env;
>      Error *local_err = NULL;
>
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_RISCV_CPU_HOST)) {
> +        error_setg(errp, "'host' CPU is not compatible with TCG acceleration");
> +        return;
> +    }
> +
>      riscv_cpu_validate_misa_mxl(cpu, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
> --
> 2.41.0
>
>

Re: [PATCH] target/riscv/cpu.c: do not run 'host' CPU with TCG

Posted by Alistair Francis 9 months, 1 week ago

On Fri, Jul 21, 2023 at 11:35 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> The 'host' CPU is available in a CONFIG_KVM build and it's currently
> available for all accels, but is a KVM only CPU. This means that in a
> RISC-V KVM capable host we can do things like this:
>
> $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
> qemu-system-riscv64: H extension requires priv spec 1.12.0
>
> This CPU does not have a priv spec because we don't filter its extensions
> via priv spec. We shouldn't be reaching riscv_cpu_realize_tcg() at all
> with the 'host' CPU.
>
> We don't have a way to filter the 'host' CPU out of the available CPU
> options (-cpu help) if the build includes both KVM and TCG. What we can
> do is to error out during riscv_cpu_realize_tcg() if the user chooses
> the 'host' CPU with accel=tcg:
>
> $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
> qemu-system-riscv64: 'host' CPU is not compatible with TCG acceleration
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  target/riscv/cpu.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 6b93b04453..08db3d613f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1395,6 +1395,11 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)
>      CPURISCVState *env = &cpu->env;
>      Error *local_err = NULL;
>
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_RISCV_CPU_HOST)) {
> +        error_setg(errp, "'host' CPU is not compatible with TCG acceleration");
> +        return;
> +    }
> +
>      riscv_cpu_validate_misa_mxl(cpu, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
> --
> 2.41.0
>
>

Re: [PATCH] target/riscv/cpu.c: do not run 'host' CPU with TCG

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

On 21/7/23 15:34, Daniel Henrique Barboza wrote:
> The 'host' CPU is available in a CONFIG_KVM build and it's currently
> available for all accels, but is a KVM only CPU. This means that in a
> RISC-V KVM capable host we can do things like this:
> 
> $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
> qemu-system-riscv64: H extension requires priv spec 1.12.0
> 
> This CPU does not have a priv spec because we don't filter its extensions
> via priv spec. We shouldn't be reaching riscv_cpu_realize_tcg() at all
> with the 'host' CPU.
> 
> We don't have a way to filter the 'host' CPU out of the available CPU
> options (-cpu help) if the build includes both KVM and TCG. What we can
> do is to error out during riscv_cpu_realize_tcg() if the user chooses
> the 'host' CPU with accel=tcg:
> 
> $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
> qemu-system-riscv64: 'host' CPU is not compatible with TCG acceleration
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>   target/riscv/cpu.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 6b93b04453..08db3d613f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1395,6 +1395,11 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)

/me wonders why this method isn't used as AccelCPUClass:cpu_realizefn().

>       CPURISCVState *env = &cpu->env;
>       Error *local_err = NULL;
>   
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_RISCV_CPU_HOST)) {
> +        error_setg(errp, "'host' CPU is not compatible with TCG acceleration");
> +        return;
> +    }

We should manage that generically, likely in CPUClass.
We could define riscv CPU with a DEFINE_CPU_WITH_CLASS() macro to pass
the class_init() handler. Anyhow meanwhile your patch is sufficient, so

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] target/riscv/cpu.c: do not run 'host' CPU with TCG

Posted by Daniel Henrique Barboza 9 months, 2 weeks ago


On 7/21/23 11:37, Philippe Mathieu-Daudé wrote:
> On 21/7/23 15:34, Daniel Henrique Barboza wrote:
>> The 'host' CPU is available in a CONFIG_KVM build and it's currently
>> available for all accels, but is a KVM only CPU. This means that in a
>> RISC-V KVM capable host we can do things like this:
>>
>> $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
>> qemu-system-riscv64: H extension requires priv spec 1.12.0
>>
>> This CPU does not have a priv spec because we don't filter its extensions
>> via priv spec. We shouldn't be reaching riscv_cpu_realize_tcg() at all
>> with the 'host' CPU.
>>
>> We don't have a way to filter the 'host' CPU out of the available CPU
>> options (-cpu help) if the build includes both KVM and TCG. What we can
>> do is to error out during riscv_cpu_realize_tcg() if the user chooses
>> the 'host' CPU with accel=tcg:
>>
>> $ ./build/qemu-system-riscv64 -M virt,accel=tcg -cpu host --nographic
>> qemu-system-riscv64: 'host' CPU is not compatible with TCG acceleration
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   target/riscv/cpu.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 6b93b04453..08db3d613f 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1395,6 +1395,11 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)
> 
> /me wonders why this method isn't used as AccelCPUClass:cpu_realizefn().
> 
>>       CPURISCVState *env = &cpu->env;
>>       Error *local_err = NULL;
>> +    if (object_dynamic_cast(OBJECT(dev), TYPE_RISCV_CPU_HOST)) {
>> +        error_setg(errp, "'host' CPU is not compatible with TCG acceleration");
>> +        return;
>> +    }
> 
> We should manage that generically, likely in CPUClass.
> We could define riscv CPU with a DEFINE_CPU_WITH_CLASS() macro to pass
> the class_init() handler. Anyhow meanwhile your patch is sufficient, so
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks. I'll mark all your suggestions as todo for 8.2.


Daniel

>