[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000 |
Date: |
Wed, 24 Oct 2012 09:17:26 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2012-10-24 08:31, liu ping fan wrote:
> On Tue, Oct 23, 2012 at 5:04 PM, Jan Kiszka <address@hidden> wrote:
>> On 2012-10-22 11:23, Liu Ping Fan wrote:
>>> Use local lock to protect e1000. When calling the system function,
>>> dropping the fine lock before acquiring the big lock. This will
>>> introduce broken device state, which need extra effort to fix.
>>>
>>> Signed-off-by: Liu Ping Fan <address@hidden>
>>> ---
>>> hw/e1000.c | 24 +++++++++++++++++++++++-
>>> 1 files changed, 23 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/e1000.c b/hw/e1000.c
>>> index ae8a6c5..5eddab5 100644
>>> --- a/hw/e1000.c
>>> +++ b/hw/e1000.c
>>> @@ -85,6 +85,7 @@ typedef struct E1000State_st {
>>> NICConf conf;
>>> MemoryRegion mmio;
>>> MemoryRegion io;
>>> + QemuMutex e1000_lock;
>>>
>>> uint32_t mac_reg[0x8000];
>>> uint16_t phy_reg[0x20];
>>> @@ -223,13 +224,27 @@ static const uint32_t mac_reg_init[] = {
>>> static void
>>> set_interrupt_cause(E1000State *s, int index, uint32_t val)
>>> {
>>> + QemuThread *t;
>>> +
>>> if (val && (E1000_DEVID >= E1000_DEV_ID_82547EI_MOBILE)) {
>>> /* Only for 8257x */
>>> val |= E1000_ICR_INT_ASSERTED;
>>> }
>>> s->mac_reg[ICR] = val;
>>> s->mac_reg[ICS] = val;
>>> - qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) != 0);
>>> +
>>> + t = pthread_getspecific(qemu_thread_key);
>>> + if (t->context_type == 1) {
>>> + qemu_mutex_unlock(&s->e1000_lock);
>>> + qemu_mutex_lock_iothread();
>>> + }
>>> + if (DEVICE(s)->state < DEV_STATE_STOPPING) {
>>> + qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) !=
>>> 0);
>>> + }
>>> + if (t->context_type == 1) {
>>> + qemu_mutex_unlock_iothread();
>>> + qemu_mutex_lock(&s->e1000_lock);
>>> + }
>>
>> This is ugly for many reasons. First of all, it is racy as the register
>> content may change while dropping the device lock, no? Then you would
>> raise or clear an IRQ spuriously.
>>
> Device state's intact is protected by busy flag, and will not broken
Except that the busy flag concept is broken in itself.
I see that we have a all-or-nothing problem here: to address this
properly, we need to convert the IRQ path to lock-less (or at least
compatible with holding per-device locks) as well.
Jan
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, (continued)
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, liu ping fan, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Paolo Bonzini, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Peter Maydell, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Jan Kiszka, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Paolo Bonzini, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Peter Maydell, 2012/10/23
[Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Liu Ping Fan, 2012/10/22
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/22
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/23
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, liu ping fan, 2012/10/24
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000,
Jan Kiszka <=
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, liu ping fan, 2012/10/29
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, liu ping fan, 2012/10/24
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25