Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state

[liu ping fan]

On Tue, Oct 23, 2012 at 5:37 PM, Avi Kivity <address@hidden> wrote:
> On 10/23/2012 11:32 AM, liu ping fan wrote:
>> On Tue, Oct 23, 2012 at 5:07 PM, Jan Kiszka <address@hidden> wrote:
>>> On 2012-10-23 07:52, liu ping fan wrote:
>>>> On Mon, Oct 22, 2012 at 6:40 PM, Avi Kivity <address@hidden> wrote:
>>>>> On 10/22/2012 11:23 AM, Liu Ping Fan wrote:
>>>>>> The broken device state is caused by releasing local lock before 
>>>>>> acquiring
>>>>>> big lock. To fix this issue, we have two choice:
>>>>>>   1.use busy flag to protect the state
>>>>>>     The drawback is that we will introduce independent busy flag for each
>>>>>>     independent device's logic unit.
>>>>>>   2.reload the device's state
>>>>>>     The drawback is if the call chain is too deep, the action to reload 
>>>>>> will
>>>>>>     touch each layer. Also the reloading means to recaculate the 
>>>>>> intermediate
>>>>>>     result based on device's regs.
>>>>>>
>>>>>> This patch adopt the solution 1 to fix the issue.
>>>>>
>>>>> Doesn't the nested mmio patch detect this?
>>>>>
>>>> It will only record and fix the issue on one thread. But guest can
>>>> touch the emulated device on muti-threads.
>>>
>>> Sorry, what does that mean? A second VCPU accessing the device will
>>> simply be ignored when it races with another VCPU? Specifically
>>>
>> Yes, just ignored.  For device which support many logic in parallel,
>> it should use independent busy flag for each logic
>
> We don't actually know that e1000 doesn't.  Why won't writing into
> different registers in parallel work?
>
I think e1000 has only one transfer logic, so one busy flag is enough.
And the normal guest's driver will access the registers one by one.
But anyway, it may have parallel modules.  So what about model it like
this
if busy:
  wait

clear busy:
   wakeup

Regards,
pingfan
>
> --
> error compiling committee.c: too many arguments to function