[PATCH 1/4] target/riscv/kvm: set 'aia_mode' to default in error path

Daniel Henrique Barboza posted 4 patches 4 weeks ago
[PATCH 1/4] target/riscv/kvm: set 'aia_mode' to default in error path
Posted by Daniel Henrique Barboza 4 weeks ago
When failing to set the selected AIA mode, 'aia_mode' is left untouched.
This means that 'aia_mode' will not reflect the actual AIA mode,
retrieved in 'default_aia_mode',

This is benign for now, but it will impact QMP query commands that will
expose the 'aia_mode' value, retrieving the wrong value.

Set 'aia_mode' to 'default_aia_mode' if we fail to change the AIA mode
in KVM.

While we're at it, rework the log/warning messages to be a bit less
verbose. Instead of:

KVM AIA: default mode is emul
qemu-system-riscv64: warning: KVM AIA: failed to set KVM AIA mode

We can use a single warning message:

qemu-system-riscv64: warning: KVM AIA: failed to set KVM AIA mode 'auto', using default host mode 'emul'

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

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 341af901c5..970a7ab2f1 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -1711,18 +1711,26 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
         error_report("KVM AIA: failed to get current KVM AIA mode");
         exit(1);
     }
-    qemu_log("KVM AIA: default mode is %s\n",
-             kvm_aia_mode_str(default_aia_mode));
 
-    if (default_aia_mode != aia_mode) {
+    if (default_aia_mode == aia_mode) {
+        qemu_log("KVM AIA: using default host mode '%s'\n",
+                  kvm_aia_mode_str(default_aia_mode));
+    } else {
         ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
                                 KVM_DEV_RISCV_AIA_CONFIG_MODE,
                                 &aia_mode, true, NULL);
-        if (ret < 0)
-            warn_report("KVM AIA: failed to set KVM AIA mode");
-        else
-            qemu_log("KVM AIA: set current mode to %s\n",
+        if (ret < 0) {
+            warn_report("KVM AIA: failed to set KVM AIA mode '%s', using "
+                        "default host mode '%s'",
+                        kvm_aia_mode_str(aia_mode),
+                        kvm_aia_mode_str(default_aia_mode));
+
+            /* failed to change AIA mode, use default */
+            aia_mode = default_aia_mode;
+        } else {
+            qemu_log("KVM AIA: setting current mode to %s\n",
                      kvm_aia_mode_str(aia_mode));
+        }
     }
 
     ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
-- 
2.45.2
Re: [PATCH 1/4] target/riscv/kvm: set 'aia_mode' to default in error path
Posted by Alistair Francis 1 week, 4 days ago
On Tue, Sep 24, 2024 at 10:46 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> When failing to set the selected AIA mode, 'aia_mode' is left untouched.
> This means that 'aia_mode' will not reflect the actual AIA mode,
> retrieved in 'default_aia_mode',
>
> This is benign for now, but it will impact QMP query commands that will
> expose the 'aia_mode' value, retrieving the wrong value.
>
> Set 'aia_mode' to 'default_aia_mode' if we fail to change the AIA mode
> in KVM.
>
> While we're at it, rework the log/warning messages to be a bit less
> verbose. Instead of:
>
> KVM AIA: default mode is emul
> qemu-system-riscv64: warning: KVM AIA: failed to set KVM AIA mode
>
> We can use a single warning message:
>
> qemu-system-riscv64: warning: KVM AIA: failed to set KVM AIA mode 'auto', using default host mode 'emul'
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  target/riscv/kvm/kvm-cpu.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 341af901c5..970a7ab2f1 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1711,18 +1711,26 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift,
>          error_report("KVM AIA: failed to get current KVM AIA mode");
>          exit(1);
>      }
> -    qemu_log("KVM AIA: default mode is %s\n",
> -             kvm_aia_mode_str(default_aia_mode));
>
> -    if (default_aia_mode != aia_mode) {
> +    if (default_aia_mode == aia_mode) {
> +        qemu_log("KVM AIA: using default host mode '%s'\n",
> +                  kvm_aia_mode_str(default_aia_mode));
> +    } else {
>          ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
>                                  KVM_DEV_RISCV_AIA_CONFIG_MODE,
>                                  &aia_mode, true, NULL);
> -        if (ret < 0)
> -            warn_report("KVM AIA: failed to set KVM AIA mode");
> -        else
> -            qemu_log("KVM AIA: set current mode to %s\n",
> +        if (ret < 0) {
> +            warn_report("KVM AIA: failed to set KVM AIA mode '%s', using "
> +                        "default host mode '%s'",
> +                        kvm_aia_mode_str(aia_mode),
> +                        kvm_aia_mode_str(default_aia_mode));
> +
> +            /* failed to change AIA mode, use default */
> +            aia_mode = default_aia_mode;
> +        } else {
> +            qemu_log("KVM AIA: setting current mode to %s\n",
>                       kvm_aia_mode_str(aia_mode));
> +        }
>      }
>
>      ret = kvm_device_access(aia_fd, KVM_DEV_RISCV_AIA_GRP_CONFIG,
> --
> 2.45.2
>
>