[PATCH 0/2] i386/kvm: Cleanups in kvm_arch_init()

[PATCH 0/2] i386/kvm: Cleanups in kvm_arch_init()

[Xiaoyao Li]

Patch 1 removes the object_dynamic_cast() check in kvm_arch_init();

Patch 2 removes the unncessary kvm_check_extension(s, KVM_CAP_X86_SMM);

Xiaoyao Li (2):
  i386/kvm: Get X86MachineState in kvm_arch_init() without the cast
    check
  i386/kvm: Drop KVM_CAP_X86_SMM check in kvm_arch_init()

 target/i386/kvm/kvm.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

-- 
2.43.0

[PATCH 1/2] i386/kvm: Get X86MachineState in kvm_arch_init() without the cast check

[Xiaoyao Li]

Commit 8f54bbd0b4d9 ("x86: Check for machine state object class before
typecasting it") added back the object_dynamic_cast() check before
casting MachineState to X86MachineState. And commit 035d1ef26565 ("i386:
Add ratelimit for bus locks acquired in guest") followed it.

The reason to check object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE)
before commit 8f54bbd0b4d9 was that smm was not supported for microvm
machine at that time. But after commit 8f54bbd0b4d9, smm is supported
for all x86 machines (both pc and microvm). And since it's the
target-specifc implementation of kvm_arch_init() in target/i386/kvm/kvm.c,
I don't see how it would be called for other machines than x86machine,
and why the check of object_dynamic_cast() is needed.

Drop the object_dynamic_cast() check and simplify the code.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/kvm/kvm.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 369626f8c8d7..d145ad49e4e5 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -3230,6 +3230,7 @@ static int kvm_vm_enable_energy_msrs(KVMState *s)
 
 int kvm_arch_init(MachineState *ms, KVMState *s)
 {
+    X86MachineState *x86ms = X86_MACHINE(ms);
     int ret;
     struct utsname utsname;
     Error *local_err = NULL;
@@ -3311,8 +3312,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     }
 
     if (kvm_check_extension(s, KVM_CAP_X86_SMM) &&
-        object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE) &&
-        x86_machine_is_smm_enabled(X86_MACHINE(ms))) {
+        x86_machine_is_smm_enabled(x86ms)) {
         smram_machine_done.notify = register_smram_listener;
         qemu_add_machine_init_done_notifier(&smram_machine_done);
     }
@@ -3326,18 +3326,14 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         }
     }
 
-    if (object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
-        X86MachineState *x86ms = X86_MACHINE(ms);
-
-        if (x86ms->bus_lock_ratelimit > 0) {
-            ret = kvm_vm_enable_bus_lock_exit(s);
-            if (ret < 0) {
-                return ret;
-            }
-            ratelimit_init(&bus_lock_ratelimit_ctrl);
-            ratelimit_set_speed(&bus_lock_ratelimit_ctrl,
-                                x86ms->bus_lock_ratelimit, BUS_LOCK_SLICE_TIME);
+    if (x86ms->bus_lock_ratelimit > 0) {
+        ret = kvm_vm_enable_bus_lock_exit(s);
+        if (ret < 0) {
+            return ret;
         }
+        ratelimit_init(&bus_lock_ratelimit_ctrl);
+        ratelimit_set_speed(&bus_lock_ratelimit_ctrl,
+                            x86ms->bus_lock_ratelimit, BUS_LOCK_SLICE_TIME);
     }
 
     if (kvm_check_extension(s, KVM_CAP_X86_NOTIFY_VMEXIT)) {
-- 
2.43.0

[PATCH 2/2] i386/kvm: Drop KVM_CAP_X86_SMM check in kvm_arch_init()

[Xiaoyao Li]

x86_machine_is_smm_enabled() checks the KVM_CAP_X86_SMM for KVM
case. No need to check KVM_CAP_X86_SMM specifically.

One benefit of checking it specifically is that it can return early when
KVM_CAP_X86_SMM is not supported. But considering KVM_CAP_X86_SMM was
added in KVM 10 year ago, it's unlikely the case.

So just drop the check of KVM_CAP_X86_SMM to simplify the code.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 target/i386/kvm/kvm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index d145ad49e4e5..92025b8fcae9 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -3311,8 +3311,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         return ret;
     }
 
-    if (kvm_check_extension(s, KVM_CAP_X86_SMM) &&
-        x86_machine_is_smm_enabled(x86ms)) {
+    if (x86_machine_is_smm_enabled(x86ms)) {
         smram_machine_done.notify = register_smram_listener;
         qemu_add_machine_init_done_notifier(&smram_machine_done);
     }
-- 
2.43.0

Re: [PATCH 1/2] i386/kvm: Get X86MachineState in kvm_arch_init() without the cast check

[Philippe Mathieu-Daudé]

On 29/7/25 08:20, Xiaoyao Li wrote:
> Commit 8f54bbd0b4d9 ("x86: Check for machine state object class before
> typecasting it") added back the object_dynamic_cast() check before
> casting MachineState to X86MachineState. And commit 035d1ef26565 ("i386:
> Add ratelimit for bus locks acquired in guest") followed it.
> 
> The reason to check object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE)
> before commit 8f54bbd0b4d9 was that smm was not supported for microvm
> machine at that time. But after commit 8f54bbd0b4d9, smm is supported
> for all x86 machines (both pc and microvm). And since it's the
> target-specifc implementation of kvm_arch_init() in target/i386/kvm/kvm.c,
> I don't see how it would be called for other machines than x86machine,
> and why the check of object_dynamic_cast() is needed.
> 
> Drop the object_dynamic_cast() check and simplify the code.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>   target/i386/kvm/kvm.c | 22 +++++++++-------------
>   1 file changed, 9 insertions(+), 13 deletions(-)

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

Re: [PATCH 1/2] i386/kvm: Get X86MachineState in kvm_arch_init() without the cast check

[Chenyi Qiang]

On 7/29/2025 2:20 PM, Xiaoyao Li wrote:
> Commit 8f54bbd0b4d9 ("x86: Check for machine state object class before
> typecasting it") added back the object_dynamic_cast() check before
> casting MachineState to X86MachineState. And commit 035d1ef26565 ("i386:
> Add ratelimit for bus locks acquired in guest") followed it.
> 
> The reason to check object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE)
> before commit 8f54bbd0b4d9 was that smm was not supported for microvm
> machine at that time. But after commit 8f54bbd0b4d9, smm is supported
> for all x86 machines (both pc and microvm). And since it's the
> target-specifc implementation of kvm_arch_init() in target/i386/kvm/kvm.c,
> I don't see how it would be called for other machines than x86machine,
> and why the check of object_dynamic_cast() is needed.
> 
> Drop the object_dynamic_cast() check and simplify the code.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>

LGTM.

Reviewed-by: Chenyi Qiang <chenyi.qiang@intel.com>

> ---
>  target/i386/kvm/kvm.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 369626f8c8d7..d145ad49e4e5 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -3230,6 +3230,7 @@ static int kvm_vm_enable_energy_msrs(KVMState *s)
>  
>  int kvm_arch_init(MachineState *ms, KVMState *s)
>  {
> +    X86MachineState *x86ms = X86_MACHINE(ms);
>      int ret;
>      struct utsname utsname;
>      Error *local_err = NULL;
> @@ -3311,8 +3312,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>      }
>  
>      if (kvm_check_extension(s, KVM_CAP_X86_SMM) &&
> -        object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE) &&
> -        x86_machine_is_smm_enabled(X86_MACHINE(ms))) {
> +        x86_machine_is_smm_enabled(x86ms)) {
>          smram_machine_done.notify = register_smram_listener;
>          qemu_add_machine_init_done_notifier(&smram_machine_done);
>      }
> @@ -3326,18 +3326,14 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>          }
>      }
>  
> -    if (object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
> -        X86MachineState *x86ms = X86_MACHINE(ms);
> -
> -        if (x86ms->bus_lock_ratelimit > 0) {
> -            ret = kvm_vm_enable_bus_lock_exit(s);
> -            if (ret < 0) {
> -                return ret;
> -            }
> -            ratelimit_init(&bus_lock_ratelimit_ctrl);
> -            ratelimit_set_speed(&bus_lock_ratelimit_ctrl,
> -                                x86ms->bus_lock_ratelimit, BUS_LOCK_SLICE_TIME);
> +    if (x86ms->bus_lock_ratelimit > 0) {
> +        ret = kvm_vm_enable_bus_lock_exit(s);
> +        if (ret < 0) {
> +            return ret;
>          }
> +        ratelimit_init(&bus_lock_ratelimit_ctrl);
> +        ratelimit_set_speed(&bus_lock_ratelimit_ctrl,
> +                            x86ms->bus_lock_ratelimit, BUS_LOCK_SLICE_TIME);
>      }
>  
>      if (kvm_check_extension(s, KVM_CAP_X86_NOTIFY_VMEXIT)) {

Re: [PATCH 0/2] i386/kvm: Cleanups in kvm_arch_init()

[Xiaoyao Li]

On 7/29/2025 2:20 PM, Xiaoyao Li wrote:
> Patch 1 removes the object_dynamic_cast() check in kvm_arch_init();
> 
> Patch 2 removes the unncessary kvm_check_extension(s, KVM_CAP_X86_SMM);

Gentle ping.

> Xiaoyao Li (2):
>    i386/kvm: Get X86MachineState in kvm_arch_init() without the cast
>      check
>    i386/kvm: Drop KVM_CAP_X86_SMM check in kvm_arch_init()
> 
>   target/i386/kvm/kvm.c | 23 +++++++++--------------
>   1 file changed, 9 insertions(+), 14 deletions(-)
>