| Message ID | 20230721133411.474105-1-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv/cpu.c: do not run 'host' CPU with TCG | expand |
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>
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 >
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 > >
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 > >
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);
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(+)