Re: [Qemu-devel] [PULL 13/21] apic_common: vapic_paddr synchronization fix

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo Bonzini
> Il 19/09/2014 12:43, Pavel Dovgaluk ha scritto:
> > I've tested this patch with replay. I enabled VM reset (which was 
> > previously disabled for
> replay)
> > while loading the VM state and discovered the following problem.
> > vapic_enable function in kvmapic.c retrieves cpu number with the 
> > get_kpcr_number() function.
> > When cpu number is -1 vapic_enable exits and does not call 
> > apic_enable_vapic, which should
> > setup vapic_paddr field.
> > Without this call vapic_paddr remains initialized with default value and 
> > behavior of the
> virtual
> > machine becomes different.
> 
> IIUC the fix would be to move part of vapic_enable out to its separate
> function, and call it from do_vapic_enable?  Could you prepare a patch?

static int vapic_enable(VAPICROMState *s, X86CPU *cpu)
{
    int cpu_number = get_kpcr_number(cpu);
    hwaddr vapic_paddr;
    static const uint8_t enabled = 1;

    if (cpu_number < 0) {
        return -1;
    }
    vapic_paddr = s->vapic_paddr +
        (((hwaddr)cpu_number) << VAPIC_CPU_SHIFT);
    cpu_physical_memory_write(vapic_paddr + offsetof(VAPICState, enabled),
                              &enabled, sizeof(enabled));
    apic_enable_vapic(cpu->apic_state, vapic_paddr);

    s->state = VAPIC_ACTIVE;

    return 0;
}

vapic_paddr depends on cpu_number. cpu_number cannot be retrieved when 
do_vapic_enable executes.
Thus we cannot reconstruct vapic_paddr in that function.

Pavel Dovgalyuk