On Thu, Oct 08, 2009 at 04:09:27PM +0200, Avi Kivity wrote:
> On 10/08/2009 03:55 PM, Anthony Liguori wrote:
>> Glauber Costa wrote:
>>> This patch provides kvm with an in-kernel apic. We are currently not
>>> enabling it.
>>> The code is heavily based on what's in qemu-kvm.git.
>>>
>>> Signed-off-by: Glauber Costa <glommer@xxxxxxxxxx>
>>> ---
>>> hw/apic.c | 135
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++--
>>> kvm.h | 3 +
>>> target-i386/kvm.c | 18 +++++++
>>> 3 files changed, 152 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/apic.c b/hw/apic.c
>>> index c89008e..5635607 100644
>>> --- a/hw/apic.c
>>> +++ b/hw/apic.c
>>> @@ -299,7 +299,11 @@ void cpu_set_apic_base(CPUState *env, uint64_t val)
>>> #endif
>>> if (!s)
>>> return;
>>> - s->apicbase = (val & 0xfffff000) |
>>> +
>>> + if (kvm_enabled() && kvm_irqchip_in_kernel())
>>> + s->apicbase = val;
>>> + else
>>> + s->apicbase = (val & 0xfffff000) |
>>> (s->apicbase & (MSR_IA32_APICBASE_BSP |
>>> MSR_IA32_APICBASE_ENABLE));
>>> /* if disabled, cannot be enabled again */
>>> if (!(val & MSR_IA32_APICBASE_ENABLE)) {
>>> @@ -497,6 +501,13 @@ void apic_init_reset(CPUState *env)
>>> s->wait_for_sipi = 1;
>>>
>>> env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
>>> +
>>> +#ifdef KVM_CAP_MP_STATE
>>> + if (kvm_enabled() && kvm_irqchip_in_kernel())
>>> + env->mp_state
>>> + = env->halted ? KVM_MP_STATE_UNINITIALIZED :
>>> KVM_MP_STATE_RUNNABLE;
>>> +#endif
>>
>> I don't think CAP_MP_STATE should be treated as an optional feature.
>>
>>> +static int kvm_kernel_lapic_load_from_user(APICState *s)
>>> +{
>>> + int r = 0;
>>> +#if defined(KVM_CAP_IRQCHIP) && defined(TARGET_I386)
>>> + struct kvm_lapic_state apic;
>>> + struct kvm_lapic_state *klapic = &apic;
>>> + int i;
>>> +
>>> + if (!(kvm_enabled() && kvm_irqchip_in_kernel()))
>>> + return 0;
>>> +
>>> + memset(klapic, 0, sizeof apic);
>>> + kapic_set_reg(klapic, 0x2, s->id << 24);
>>> + kapic_set_reg(klapic, 0x8, s->tpr);
>>> + kapic_set_reg(klapic, 0xd, s->log_dest << 24);
>>> + kapic_set_reg(klapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
>>> + kapic_set_reg(klapic, 0xf, s->spurious_vec);
>>> + for (i = 0; i < 8; i++) {
>>> + kapic_set_reg(klapic, 0x10 + i, s->isr[i]);
>>> + kapic_set_reg(klapic, 0x18 + i, s->tmr[i]);
>>> + kapic_set_reg(klapic, 0x20 + i, s->irr[i]);
>>> + }
>>> + kapic_set_reg(klapic, 0x28, s->esr);
>>> + kapic_set_reg(klapic, 0x30, s->icr[0]);
>>> + kapic_set_reg(klapic, 0x31, s->icr[1]);
>>> + for (i = 0; i < APIC_LVT_NB; i++)
>>> + kapic_set_reg(klapic, 0x32 + i, s->lvt[i]);
>>> + kapic_set_reg(klapic, 0x38, s->initial_count);
>>> + kapic_set_reg(klapic, 0x3e, s->divide_conf);
>>> +
>>> + r = kvm_set_lapic(s->cpu_env, klapic);
>>> +#endif
>>> + return r;
>>> +}
>>
>> You should probably just setup VMState such that it directly saves
>> kvm_lapic_state and then have the pre/post functions call the kernel
>> ioctls to sync it. There's not a whole lot of point switching the
>> state between two different structures.
>
> It ensures the two models are compatible. Since they're the same device
> from the point of view of the guest, there's no reason for them to have
> different representations or to be incompatible.
live migration between something that has in-kernel irqchip and something that
has not is likely to be a completely untested thing. And this is the only reason
we might think of it as the same device. I don't see any value in supporting this
combination
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
