Re: [PATCH 2/2] qemu-kvm: x86: Add support for VCPU event states

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2171 bytes --]

Avi Kivity wrote:
> On 11/15/2009 04:21 PM, Jan Kiszka wrote:
>> Avi Kivity wrote:
>>   
>>> On 11/12/2009 02:05 AM, Jan Kiszka wrote:
>>>     
>>>> This patch extends the qemu-kvm state sync logic with support for
>>>> KVM_GET/SET_VCPU_EVENTS, giving access to yet missing exception,
>>>> interrupt and NMI states.
>>>>
>>>> diff --git a/target-i386/machine.c b/target-i386/machine.c
>>>> index 6bd447f..1eda7c5 100644
>>>> --- a/target-i386/machine.c
>>>> +++ b/target-i386/machine.c
>>>> @@ -452,6 +452,11 @@ static const VMStateDescription vmstate_cpu = {
>>>>            VMSTATE_INT32_V(interrupt_injected, CPUState, 9),
>>>>            VMSTATE_UINT32_V(mp_state, CPUState, 9),
>>>>            VMSTATE_UINT64_V(tsc, CPUState, 9),
>>>> +        VMSTATE_UINT8_V(soft_interrupt, CPUState, 11),
>>>> +        VMSTATE_UINT8_V(nmi_injected, CPUState, 11),
>>>> +        VMSTATE_UINT8_V(nmi_pending, CPUState, 11),
>>>> +        VMSTATE_UINT8_V(has_error_code, CPUState, 11),
>>>> +        VMSTATE_UINT32_V(sipi_vector, CPUState, 11),
>>>>            /* MCE */
>>>>            VMSTATE_UINT64_V(mcg_cap, CPUState, 10),
>>>>            VMSTATE_UINT64_V(mcg_status, CPUState, 10),
>>>>
>>>>
>>>>        
>>> Is there a reason why you add 11 between 9 and 10?  We'll probably see
>>> another 11 when someone else adds the next state.
>>>
>>>      
>> Logical grouping ("/* KVM-related states */").
> 
> These aren't kvm-related, just not implemented in tcg yet.  Nothing
> kvmish about them - it's all architectural state.

Most of them are KVM-specific. TCG don't have to deal with event
re-injection due to host page faults etc. on first try.

> 
>> If anyone once tries to
>> add non-KVM stuff here just because it's version 12, it should be
>> rejected. I don't think you have to sort VMSTATE entries by their
>> version number. Am I right, Juan?
>>    
> 
> I'm worried about something else - someone looking at the end, seeing
> version 10, and appending new state with version 11.

Again, that's something proper review should catch (just like checking
for the right place when adding a new IOCTL to kvm.h).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]