diff mbox series

[v2] target/riscv: Restrict KVM-specific fields from ArchCPU

Message ID 20230624192957.14067-1-philmd@linaro.org
State New
Headers show
Series [v2] target/riscv: Restrict KVM-specific fields from ArchCPU | expand

Commit Message

Philippe Mathieu-Daudé June 24, 2023, 7:29 p.m. UTC 
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(-)

Comments

Daniel Henrique Barboza June 25, 2023, 11:27 p.m. UTC | #1 
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,
Philippe Mathieu-Daudé June 26, 2023, 10:56 a.m. UTC | #2 
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,
Alistair Francis July 3, 2023, 1:41 a.m. UTC | #3 
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
>
>
diff mbox series

Patch

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,