* Re: [PATCH v2] target/riscv: Restrict KVM-specific fields from ArchCPU
2023-06-24 19:29 [PATCH v2] target/riscv: Restrict KVM-specific fields from ArchCPU Philippe Mathieu-Daudé
@ 2023-06-25 23:27 ` Daniel Henrique Barboza
2023-06-26 10:56 ` Philippe Mathieu-Daudé
2023-07-03 1:41 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Henrique Barboza @ 2023-06-25 23:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Weiwei Li, Richard Henderson, qemu-riscv, Alistair Francis,
Liu Zhiwei, Palmer Dabbelt, Bin Meng
On 6/24/23 16:29, Philippe Mathieu-Daudé wrote:
> These fields shouldn't be accessed when KVM is not available.
>
> Restrict the KVM timer migration state. Rename the KVM timer
> post_load() handler accordingly, because cpu_post_load() is
> too generic.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Since v1 https://lore.kernel.org/qemu-devel/20230405160454.97436-10-philmd@linaro.org/:
> - Restrict whole vmstate_kvmtimer (thus drop Daniel's R-b)
> ---
> target/riscv/cpu.h | 2 ++
> target/riscv/machine.c | 8 ++++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e3e08d315f..b1b56aa29e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -363,12 +363,14 @@ struct CPUArchState {
> hwaddr kernel_addr;
> hwaddr fdt_addr;
>
> +#ifdef CONFIG_KVM
> /* kvm timer */
> bool kvm_timer_dirty;
> uint64_t kvm_timer_time;
> uint64_t kvm_timer_compare;
> uint64_t kvm_timer_state;
> uint64_t kvm_timer_frequency;
> +#endif /* CONFIG_KVM */
> };
>
> /*
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 3ce2970785..d27f8a2a3a 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -199,7 +199,8 @@ static bool kvmtimer_needed(void *opaque)
> return kvm_enabled();
> }
>
> -static int cpu_post_load(void *opaque, int version_id)
> +#ifdef CONFIG_KVM
> +static int cpu_kvmtimer_post_load(void *opaque, int version_id)
> {
> RISCVCPU *cpu = opaque;
> CPURISCVState *env = &cpu->env;
> @@ -213,7 +214,7 @@ static const VMStateDescription vmstate_kvmtimer = {
> .version_id = 1,
> .minimum_version_id = 1,
> .needed = kvmtimer_needed,
> - .post_load = cpu_post_load,
> + .post_load = cpu_kvmtimer_post_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> @@ -221,6 +222,7 @@ static const VMStateDescription vmstate_kvmtimer = {
> VMSTATE_END_OF_LIST()
> }
> };
> +#endif
>
> static bool debug_needed(void *opaque)
> {
> @@ -409,7 +411,9 @@ const VMStateDescription vmstate_riscv_cpu = {
> &vmstate_vector,
> &vmstate_pointermasking,
> &vmstate_rv128,
> +#ifdef CONFIG_KVM
> &vmstate_kvmtimer,
> +#endif
> &vmstate_envcfg,
> &vmstate_debug,
> &vmstate_smstateen,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] target/riscv: Restrict KVM-specific fields from ArchCPU
2023-06-24 19:29 [PATCH v2] target/riscv: Restrict KVM-specific fields from ArchCPU Philippe Mathieu-Daudé
2023-06-25 23:27 ` Daniel Henrique Barboza
@ 2023-06-26 10:56 ` Philippe Mathieu-Daudé
2023-07-03 1:41 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-06-26 10:56 UTC (permalink / raw)
To: qemu-devel
Cc: Weiwei Li, Richard Henderson, Daniel Henrique Barboza, qemu-riscv,
Alistair Francis, Liu Zhiwei, Palmer Dabbelt, Bin Meng
On 24/6/23 21:29, Philippe Mathieu-Daudé wrote:
> These fields shouldn't be accessed when KVM is not available.
>
> Restrict the KVM timer migration state. Rename the KVM timer
> post_load() handler accordingly, because cpu_post_load() is
> too generic.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Since v1 https://lore.kernel.org/qemu-devel/20230405160454.97436-10-philmd@linaro.org/:
> - Restrict whole vmstate_kvmtimer (thus drop Daniel's R-b)
> ---
> target/riscv/cpu.h | 2 ++
> target/riscv/machine.c | 8 ++++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e3e08d315f..b1b56aa29e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -363,12 +363,14 @@ struct CPUArchState {
> hwaddr kernel_addr;
> hwaddr fdt_addr;
>
> +#ifdef CONFIG_KVM
> /* kvm timer */
> bool kvm_timer_dirty;
> uint64_t kvm_timer_time;
> uint64_t kvm_timer_compare;
> uint64_t kvm_timer_state;
> uint64_t kvm_timer_frequency;
> +#endif /* CONFIG_KVM */
> };
>
> /*
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 3ce2970785..d27f8a2a3a 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
Oops, here goes ...
> @@ -199,7 +199,8 @@ static bool kvmtimer_needed(void *opaque)
> return kvm_enabled();
> }
>
> -static int cpu_post_load(void *opaque, int version_id)
> +#ifdef CONFIG_KVM
... this ^^^ line.
> +static int cpu_kvmtimer_post_load(void *opaque, int version_id)
> {
> RISCVCPU *cpu = opaque;
> CPURISCVState *env = &cpu->env;
> @@ -213,7 +214,7 @@ static const VMStateDescription vmstate_kvmtimer = {
> .version_id = 1,
> .minimum_version_id = 1,
> .needed = kvmtimer_needed,
> - .post_load = cpu_post_load,
> + .post_load = cpu_kvmtimer_post_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> @@ -221,6 +222,7 @@ static const VMStateDescription vmstate_kvmtimer = {
> VMSTATE_END_OF_LIST()
> }
> };
> +#endif
>
> static bool debug_needed(void *opaque)
> {
> @@ -409,7 +411,9 @@ const VMStateDescription vmstate_riscv_cpu = {
> &vmstate_vector,
> &vmstate_pointermasking,
> &vmstate_rv128,
> +#ifdef CONFIG_KVM
> &vmstate_kvmtimer,
> +#endif
> &vmstate_envcfg,
> &vmstate_debug,
> &vmstate_smstateen,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] target/riscv: Restrict KVM-specific fields from ArchCPU
2023-06-24 19:29 [PATCH v2] target/riscv: Restrict KVM-specific fields from ArchCPU Philippe Mathieu-Daudé
2023-06-25 23:27 ` Daniel Henrique Barboza
2023-06-26 10:56 ` Philippe Mathieu-Daudé
@ 2023-07-03 1:41 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2023-07-03 1:41 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Weiwei Li, Richard Henderson, Daniel Henrique Barboza,
qemu-riscv, Alistair Francis, Liu Zhiwei, Palmer Dabbelt,
Bin Meng
On Sun, Jun 25, 2023 at 5:30 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> These fields shouldn't be accessed when KVM is not available.
>
> Restrict the KVM timer migration state. Rename the KVM timer
> post_load() handler accordingly, because cpu_post_load() is
> too generic.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> Since v1 https://lore.kernel.org/qemu-devel/20230405160454.97436-10-philmd@linaro.org/:
> - Restrict whole vmstate_kvmtimer (thus drop Daniel's R-b)
> ---
> target/riscv/cpu.h | 2 ++
> target/riscv/machine.c | 8 ++++++--
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index e3e08d315f..b1b56aa29e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -363,12 +363,14 @@ struct CPUArchState {
> hwaddr kernel_addr;
> hwaddr fdt_addr;
>
> +#ifdef CONFIG_KVM
> /* kvm timer */
> bool kvm_timer_dirty;
> uint64_t kvm_timer_time;
> uint64_t kvm_timer_compare;
> uint64_t kvm_timer_state;
> uint64_t kvm_timer_frequency;
> +#endif /* CONFIG_KVM */
> };
>
> /*
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 3ce2970785..d27f8a2a3a 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -199,7 +199,8 @@ static bool kvmtimer_needed(void *opaque)
> return kvm_enabled();
> }
>
> -static int cpu_post_load(void *opaque, int version_id)
> +#ifdef CONFIG_KVM
> +static int cpu_kvmtimer_post_load(void *opaque, int version_id)
> {
> RISCVCPU *cpu = opaque;
> CPURISCVState *env = &cpu->env;
> @@ -213,7 +214,7 @@ static const VMStateDescription vmstate_kvmtimer = {
> .version_id = 1,
> .minimum_version_id = 1,
> .needed = kvmtimer_needed,
> - .post_load = cpu_post_load,
> + .post_load = cpu_kvmtimer_post_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> @@ -221,6 +222,7 @@ static const VMStateDescription vmstate_kvmtimer = {
> VMSTATE_END_OF_LIST()
> }
> };
> +#endif
>
> static bool debug_needed(void *opaque)
> {
> @@ -409,7 +411,9 @@ const VMStateDescription vmstate_riscv_cpu = {
> &vmstate_vector,
> &vmstate_pointermasking,
> &vmstate_rv128,
> +#ifdef CONFIG_KVM
> &vmstate_kvmtimer,
> +#endif
> &vmstate_envcfg,
> &vmstate_debug,
> &vmstate_smstateen,
> --
> 2.38.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread