Avi Kivity wrote: > On 09/23/2009 06:07 PM, Jan Kiszka wrote: >> Avi Kivity wrote: >> >>> On 09/22/2009 05:51 PM, Gleb Natapov wrote: >>> >>>> Each caller of the function already calls it. >>>> >>>> Signed-off-by: Gleb Natapov<gleb@xxxxxxxxxx> >>>> --- >>>> hw/apic.c | 1 - >>>> 1 files changed, 0 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/hw/apic.c b/hw/apic.c >>>> index 3a2e128..a9d1fb8 100644 >>>> --- a/hw/apic.c >>>> +++ b/hw/apic.c >>>> @@ -488,7 +488,6 @@ void apic_init_reset(CPUState *env) >>>> if (!s) >>>> return; >>>> >>>> - cpu_synchronize_state(env); >>>> s->tpr = 0; >>>> s->spurious_vec = 0xff; >>>> s->log_dest = 0; >>>> >>>> >>> Still, it's safer to live this in. >>> >>> >> Yet another diff to upstream... >> >> It's really time to stabilize this still a bit fuzzy register sync >> model, also in qemu-kvm. If you think we need it, let us push it >> upstream - with a sound explanation, and maybe even with more sync >> points for the sake of consistency. >> >> > > Functions calling each other in the same subsystem can rely on callers > calling cpu_synchronize_state(). Across subsystems, that's another > matter, exported functions should try not to rely on implementation > details of their callers. > > (You might argue that the apic is not separate subsystem wrt an x86 cpu, > and I'm not sure I have a counterargument) > I do accept this argument. It's just that my feeling is that we are lacking proper review of the required call sites of cpu_sychronize_state and rather put it where some regression popped up (and that only in qemu-kvm...). The new rule is: Synchronize the states before accessing registers (or in-kernel devices) the first time after a vmexit to user space. But, e.g., I do not see where we do this on CPU reset. Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
