[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/10] apic_common: vapic_paddr synchronization
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 02/10] apic_common: vapic_paddr synchronization fix |
Date: |
Tue, 09 Sep 2014 16:26:16 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.0 |
Il 09/09/2014 16:00, Michael S. Tsirkin ha scritto:
> On Tue, Sep 09, 2014 at 02:30:01PM +0200, Paolo Bonzini wrote:
>> From: Pavel Dovgalyuk <address@hidden>
>>
>> This patch postpones vapic_paddr initialization, which is performed
>> during migration. When vapic_paddr is synchronized within the migration
>> process, apic_common functions could operate with incorrect apic state,
>> if it hadn't loaded yet. This patch postpones the synchronization until
>> the virtual machine is started, ensuring that the whole virtual machine
>> state has been loaded.
>>
>> Signed-off-by: Pavel Dovgalyuk <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>> hw/i386/kvmvapic.c | 37 ++++++++++++++++++++++++++-----------
>> 1 file changed, 26 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
>> index ee95963..2bcc249 100644
>> --- a/hw/i386/kvmvapic.c
>> +++ b/hw/i386/kvmvapic.c
>> @@ -59,6 +59,7 @@ typedef struct VAPICROMState {
>> GuestROMState rom_state;
>> size_t rom_size;
>> bool rom_mapped_writable;
>> + VMChangeStateEntry *vmsentry;
>> } VAPICROMState;
>>
>> #define TYPE_VAPIC "kvmvapic"
>> @@ -734,11 +735,34 @@ static void do_vapic_enable(void *data)
>> vapic_enable(s, cpu);
>> }
>>
>> -static int vapic_post_load(void *opaque, int version_id)
>> +static void kvmvapic_vm_state_change(void *opaque, int running,
>> + RunState state)
>> {
>> VAPICROMState *s = opaque;
>> uint8_t *zero;
>>
>> + if (!running) {
>> + return;
>> + }
>> +
>> + if (s->state == VAPIC_ACTIVE) {
>> + if (smp_cpus == 1) {
>
> maybe add a comment explaining why we need
> to special case smp_cpus?
I don't know honestly. I'm just moving code around.
>> + run_on_cpu(first_cpu, do_vapic_enable, s);
>
> Is this safe to do in vmstate handler? cpu isn't running yet is it?
Yes, it isn't running and run_on_cpu is synchronous.
Paolo
>> + } else {
>> + zero = g_malloc0(s->rom_state.vapic_size);
>> + cpu_physical_memory_write(s->vapic_paddr, zero,
>> + s->rom_state.vapic_size);
>> + g_free(zero);
>> + }
>> + }
>> +
>> + qemu_del_vm_change_state_handler(s->vmsentry);
>> +}
>> +
>> +static int vapic_post_load(void *opaque, int version_id)
>> +{
>> + VAPICROMState *s = opaque;
>> +
>> /*
>> * The old implementation of qemu-kvm did not provide the state
>> * VAPIC_STANDBY. Reconstruct it.
>> @@ -752,17 +776,8 @@ static int vapic_post_load(void *opaque, int version_id)
>> return -1;
>> }
>> }
>> - if (s->state == VAPIC_ACTIVE) {
>> - if (smp_cpus == 1) {
>> - run_on_cpu(first_cpu, do_vapic_enable, s);
>> - } else {
>> - zero = g_malloc0(s->rom_state.vapic_size);
>> - cpu_physical_memory_write(s->vapic_paddr, zero,
>> - s->rom_state.vapic_size);
>> - g_free(zero);
>> - }
>> - }
>>
>> + s->vmsentry =
>> qemu_add_vm_change_state_handler(kvmvapic_vm_state_change, s);
>> return 0;
>> }
>>
>> --
>> 2.1.0
>>
>>