Re: [PULL 48/87] x86: move SMM property to X86MachineState

[Michal Prívozník]

On 12/23/19 12:33 PM, Daniel P. Berrangé wrote:
> On Mon, Dec 23, 2019 at 12:28:43PM +0100, Michal Prívozník wrote:
>> On 12/18/19 1:02 PM, Paolo Bonzini wrote:
>>> Add it to microvm as well, it is a generic property of the x86
>>> architecture.
>>>
>>> Suggested-by: Sergio Lopez <address@hidden>
>>> Signed-off-by: Paolo Bonzini <address@hidden>
>>> ---
>>>  hw/i386/pc.c          | 49 
>>> -------------------------------------------------
>>>  hw/i386/pc_piix.c     |  6 +++---
>>>  hw/i386/pc_q35.c      |  2 +-
>>>  hw/i386/x86.c         | 50 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  include/hw/i386/pc.h  |  3 ---
>>>  include/hw/i386/x86.h |  5 +++++
>>>  target/i386/kvm.c     |  3 +--
>>>  7 files changed, 59 insertions(+), 59 deletions(-)
>>>
>>
>>
>>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>>> index ef63f3a..c7ff67a 100644
>>> --- a/target/i386/kvm.c
>>> +++ b/target/i386/kvm.c
>>> @@ -2173,8 +2173,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>>      }
>>>  
>>>      if (kvm_check_extension(s, KVM_CAP_X86_SMM) &&
>>> -        object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE) &&
>>> -        pc_machine_is_smm_enabled(PC_MACHINE(ms))) {
>>> +        x86_machine_is_smm_enabled(X86_MACHINE(ms))) {
>>>          smram_machine_done.notify = register_smram_listener;
>>>          qemu_add_machine_init_done_notifier(&smram_machine_done);
>>>      }
>>>
>>
>> Sorry for not catching this earlier, but I don't think this is right.
>> The @ms is not instance of X
>>
>>
>> After I refreshed my qemu master I realized that libvirt is unable to
>> fetch capabilities. Libvirt runs the following command:
>>
>>   qemu.git $ ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config
>> -nodefaults -nographic -machine none,accel=kvm:tcg
> 
> Hmm, it would be good if we can get QEMU CI to launch QEMU  in
> this way, as this isn't the first time some change has broken
> launching of QEMU for probing capabilities.

Agreed.

NB, this diff fixes the issue for me, but I have no idea if it's correct
(it looks correct judging by the way the code looked before):

diff --git i/target/i386/kvm.c w/target/i386/kvm.c
index 0b511906e3..7ee3202634 100644
--- i/target/i386/kvm.c
+++ w/target/i386/kvm.c
@@ -2173,6 +2173,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))) {
         smram_machine_done.notify = register_smram_listener;
         qemu_add_machine_init_done_notifier(&smram_machine_done);


Michal