[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH] spapr: reset @kvm_vcpu_dirty before starting CPU
From: |
Alexey Kardashevskiy |
Subject: |
Re: [Qemu-ppc] [PATCH] spapr: reset @kvm_vcpu_dirty before starting CPU |
Date: |
Tue, 21 Jan 2014 02:02:37 +1100 |
User-agent: |
Mozilla/5.0 (X11; Linux i686 on x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 |
On 01/21/2014 01:45 AM, Alexander Graf wrote:
>
> On 15.01.2014, at 05:57, Alexey Kardashevskiy <address@hidden> wrote:
>
>> Normally QEMU kvm_arch_get_registers() reads registers and sets a dirty
>> flag which prevents further registers reading from KVM till
>> kvm_arch_put_registers() executes and resets the flag.
>>
>> However if we run QEMU with "-S" ("suspended"), then execute "info cpus"
>> from the QEMU monitor, we end up with not reading registers in
>> rtas_start_cpu() as qmp_query_cpus() calls kvm_cpu_synchronize_state()
>> which leaves @kvm_vcpu_dirty=true what prevents kvm_cpu_synchronize_state()
>> from synchronizing registers and we loose the values.
>
> But that's fine because the values are already synchronized and couldn't
> have changed because the guest cpu wasn't running, no?
Discovered today that "[PATCH] spapr: fix H_SET_MODE to sync LPCR with KVM"
seems to be v2 of this as that patch solves the issue this patch was
intended to solve so I'll drop this one.
>
>
> Alex
>
>>
>> This resets @kvm_vcpu_dirty flag as we do not have content which we really
>> want to
>> keep at this point as the CPU is halted.
>>
>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>> ---
>>
>> We could also reset @kvm_vcpu_dirty in qmp_query_cpus() but that would be
>> racy.
>> ---
>> hw/ppc/spapr_rtas.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 1cb276d..3dade5e 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -185,6 +185,8 @@ static void rtas_start_cpu(PowerPCCPU *cpu_,
>> sPAPREnvironment *spapr,
>> /* This will make sure qemu state is up to date with kvm, and
>> * mark it dirty so our changes get flushed back before the
>> * new cpu enters */
>> +
>> + cs->kvm_vcpu_dirty = false;
>> kvm_cpu_synchronize_state(cs);
>>
>> env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
>> --
>> 1.8.4.rc4
>>
>
--
Alexey