Re: [PATCH] Don't call cpu_synchronize_state() in apic_init_reset()

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

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@redhat.com>
>>>> ---
>>>>    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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]