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

[liu ping fan]

On Thu, Oct 25, 2012 at 5:04 PM, Avi Kivity <address@hidden> wrote:
> On 10/25/2012 11:00 AM, Peter Maydell wrote:
>> On 23 October 2012 10:37, 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:
>>>>>> 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?
>>
>> Unless the device we're emulating supports multiple in
>> parallel accesses (and I bet 99.9% of the devices we model
>> don't) then the memory framework needs to serialise the
>> loads/stores. Otherwise it's just going to be excessively
>> hard to write a reliable device model.
>
> That's why we have a per-device lock.  The busy flag breaks that model
> by discarding accesses that occur in parallel.
>
I think by adopting the model, we can avoid this.

struct device_logic {
  bool busy;
  qemu_mutex lock;
  QemuCond wait;
};

LOCK(logic->lock)
while (logic->busy) {
qemu_cond_wait(&logic->wait, &logic->lock);
}
....
do hardware emulation
...
logic->busy = false;
qemu_cond_signal(&logic->wait);

This is identical to the biglock's behavior for parallel access to
device for nowadays. And then, the problem left is what level for
parallel we want. If we expect more parallel, we need to degrade the
device into more logic unit.


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