On 04.01.2013, at 12:01, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:ag...@suse.de] >> Sent: Friday, January 04, 2013 4:07 PM >> To: Bhushan Bharat-R65777 >> Cc: Marcelo Tosatti; jjhe...@linux.vnet.ibm.com; Christian Borntraeger; >> Anthony >> Liguori; qemu-devel@nongnu.org qemu-devel >> Subject: Re: [Qemu-devel] [PATCH 7/7] KVM regsync: Fix >> do_kvm_cpu_synchronize_state data integrity issue >> >> >> On 04.01.2013, at 11:32, Bhushan Bharat-R65777 wrote: >> >>> >>> >>>> -----Original Message----- >>>>>>> >>>>>>> Int timer_func(CPUxxState env) >>>>>>> { >>>>>>> Struct timer_regs; >>>>>>> read_regs_type((env, &timer_regs,TIMER_REGS); //update >>>>>>> env->timer_registers Write_regs_type(env, TIMER_REGS) } >>>>>>> >>>>>>> This will get one type of register_types and can cause multiple >>>>>>> IOCTL per >>>>>> entry to QEMU. >>>>>> >>>>>> This is still too explicit. How about >>>>>> >>>>>> static inline void ppc_set_tsr(CPUState *env, target_ulong val) { >>>>>> env->kvm_sync_extra |= SYNC_TIMER_REGS; >>>>>> cpu_synchronize_registers(env); >>>>>> env->spr[SPR_TSR] = val; >>>>> >>>>> You also want env->kvm_sync_dirty also, right? >>>> >>>> Not quite, since SYNC_TIMER_REGS spans more than only TSR. So we need >>>> to make sure we fetch the non-TSR register values before we can >>>> declare TIMER_REGS as dirty. >>> >>> Right , I actually want communicate mark dirty the TIMER_REGS only :) >> >> Imagine TIMER_REGS includes TSR and TCR. Then >> >> ppc_set_tsr(); >> >> should still work without a respective ppc_set_tcr() call. So we can't just >> mark >> it dirty here. The only way we could optimize this bit would be by either >> splitting the bitmap per register or by extending the setter functions to the >> full scope of the sync: > > Are you talking about GET all register of a type, say TIMER (TCR and TSR) on > first call to cpu_synchronization_function but SET will be only for dirty > register. Say if TSR is changed then setter will only communicate that TSR is > changed? > > Or > > ( > get multiple regs of a type , say TIMER (TCR and TSR), change one or more and > mark dirty if any one changed. Set all registers of a type, TIMER (TCR and > TSR) if dirty. > In this case the first call by ppc_set_tsr() to cpu_synchronization_function > will implicitly get all registers of TIMER (TCR and TSR) - means one or more > registers of timer can be updated by QEMU. This function will update only > env->TSR. > Later if ppc_set_tcr() is called, it will also call > cpu_synchronization_function but this will see that TIMER registers are > already available to QEMU, so it will not fetch again. This function will > update the TCR also. > Finally they all, TIMER (TCR and TSR) will be pushed to kernel.
Yes :) Alex
