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...@intel.com>
LGTM.
Reviewed-by: Chenyi Qiang <chenyi.qi...@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)) {
